Skip to content

[Fix-15886] Fix thread leak in NacosRegistry using Double-Checked Locking #15915

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
QiuYucheng2003 wants to merge 3 commits into
base: 3.3
Choose a base branch
from
+24 −20
Open

[Fix-15886] Fix thread leak in NacosRegistry using Double-Checked Locking #15915

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ccd58c4
[Fix-15886] Fix thread leak in NacosRegistry using Double-Checked Loc…
Dec 24, 2025
502e794
Merge branch '3.3' into fix-15886
QiuYucheng2003 Dec 24, 2025
cb26c94
Merge branch '3.3' into fix-15886
QiuYucheng2003 Jan 9, 2026
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
44 changes: 24 additions & 20 deletions ...try/dubbo-registry-nacos/src/main/java/org/apache/dubbo/registry/nacos/NacosRegistry.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -473,26 +473,30 @@ private boolean isAdminProtocol(URL url) {

private void scheduleServiceNamesLookup(final URL url, final NacosAggregateListener listener) {
if (scheduledExecutorService == null) {
scheduledExecutorService = Executors.newSingleThreadScheduledExecutor();
scheduledExecutorService.scheduleAtFixedRate(
() -> {
Set<String> serviceNames = getAllServiceNames();
filterData(serviceNames, serviceName -> {
boolean accepted = false;
for (String category : ALL_SUPPORTED_CATEGORIES) {
String prefix = category + SERVICE_NAME_SEPARATOR;
if (serviceName != null && serviceName.startsWith(prefix)) {
accepted = true;
break;
}
}
return accepted;
});
doSubscribe(url, listener, serviceNames);
},
LOOKUP_INTERVAL,
LOOKUP_INTERVAL,
TimeUnit.SECONDS);
synchronized (this) {
if (scheduledExecutorService == null) {
scheduledExecutorService = Executors.newSingleThreadScheduledExecutor();
Copy link

Copilot AI Dec 24, 2025

Choose a reason for hiding this comment

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

Potential race condition. This assignment to scheduledExecutorService is visible to other threads before the subsequent statements are executed.

Copilot uses AI. Check for mistakes.
Copy link
Author

@QiuYucheng2003 QiuYucheng2003 Jan 9, 2026

Choose a reason for hiding this comment

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

Verified that scheduledExecutorService is declared as volatile, so this DCL pattern is thread-safe.

scheduledExecutorService.scheduleAtFixedRate(
() -> {
Set<String> serviceNames = getAllServiceNames();
filterData(serviceNames, serviceName -> {
boolean accepted = false;
for (String category : ALL_SUPPORTED_CATEGORIES) {
String prefix = category + SERVICE_NAME_SEPARATOR;
if (serviceName != null && serviceName.startsWith(prefix)) {
accepted = true;
break;
}
}
return accepted;
});
doSubscribe(url, listener, serviceNames);
},
LOOKUP_INTERVAL,
LOOKUP_INTERVAL,
TimeUnit.SECONDS);
}
}
}
}

Expand Down
Loading