Skip to content

Update ExtensionRepository.java #569

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
peterxiemin wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Update ExtensionRepository.java #569

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...onent-extension-starter/src/main/java/com/alibaba/cola/extension/ExtensionRepository.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ public Map<ExtensionCoordinate, ExtensionPointI> getExtensionRepo() {
return extensionRepo;
}

private Map<ExtensionCoordinate, ExtensionPointI> extensionRepo = new HashMap<>();
private Map<ExtensionCoordinate, ExtensionPointI> extensionRepo = new ConcurrentHashMap<>();

@significantfrank significantfrank Apr 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟠 Major: 仅将 HashMap 替换为 ConcurrentHashMap 并不能保证线程安全。ExtensionRegister 中的写入路径是 check-then-act 模式：先 put 再检查 preVal != null 来检测重复，但 ConcurrentHashMap.put() 本身不是原子的 check-then-act 操作——两个线程可能同时 put 成功并都看到 preVal == null，导致重复注册被漏检。

建议方案：

  1. 使用 ConcurrentHashMap + putIfAbsent 替代 put + 检查返回值，在 ExtensionRegister 中改为：ExtensionPointI preVal = extensionRepository.getExtensionRepo().putIfAbsent(extensionCoordinate, extensionObject);
  2. 或者，若注册只在 Spring @PostConstruct 阶段单线程执行，运行时只读，则 HashMap 已足够，加 volatile 或改 ConcurrentHashMap 反而增加不必要的复杂度。请确认实际并发场景。

@significantfrank significantfrank Apr 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Minor: ConcurrentHashMap 不允许 null key 或 null value，而 HashMap 允许。当前 ExtensionCoordinate 作为 key 应该不会为 null，但请确认所有注册路径不会传入 null value，否则运行时将从静默接受变为抛出 NullPointerException。

@significantfrank significantfrank Apr 17, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔵 Suggestion: getExtensionRepo() 直接暴露内部可变 Map 的引用，外部代码可以直接修改 Map 内容，破坏封装性。建议：

  1. 若运行时只读，返回 Collections.unmodifiableMap(extensionRepo)
  2. 或提供 get(ExtensionCoordinate) 方法，避免暴露整个 Map
  3. 若确实需要外部写入（如 ExtensionRegister），考虑提供专门的 register/putIfAbsent 方法



}