apache / apache/dubbo

[Bug] Thread leak in NacosRegistry.scheduleServiceNamesLookup due to race condition

Open
#15,886 2 comments 0 reactions 0 assignees View on GitHub
help wanted
Dominant language
Java
Stars
41.6k
Forks
26.4k
Avg merge
15h 13m
Merged PRs (30d)
4

Description

### Pre-check

- [x] I am sure that all the content I provide is in English.

### Search before asking

- [x] I had searched in the [issues](https://github.com/apache/dubbo/issues?q=is%3Aissue) and found no similar issues.

### Apache Dubbo Component

Java SDK (apache/dubbo)

### Dubbo Version

Dubbo 3.3.x (Confirmed in latest master branch) & Dubbo 2.7.x.
JDK 1.8+, All Operating Systems.

### Steps to reproduce this issue

1. This issue is identified via static code analysis (concurrency safety check).
2. It is located in `org.apache.dubbo.registry.nacos.NacosRegistry` method `scheduleServiceNamesLookup`.
3. The method uses a "Check-Then-Act" pattern to lazily initialize the `scheduledExecutorService`.
4. Although the field is `volatile`, the initialization block is NOT synchronized.

Code Analysis:
When `scheduleServiceNamesLookup` is called concurrently (e.g. via admin protocol), a race condition occurs:
if (scheduledExecutorService == null) { // Thread A and B can pass here simultaneously
scheduledExecutorService = Executors.newSingleThreadScheduledExecutor(); // Executed twice
// ...
}

### What you expected to happen

The `scheduledExecutorService` should be initialized as a Singleton in a thread-safe manner (e.g., using Double-Checked Locking). Only one thread pool should be created to avoid thread leaks.

### Anything else

### Proposed Solution (Double-Checked Locking)

I suggest modifying the code to use synchronized DCL:

private void scheduleServiceNamesLookup(final URL url, final NacosAggregateListener listener) {
if (scheduledExecutorService == null) {
synchronized (this) {
if (scheduledExecutorService == null) {
// Recommend naming the thread for better debuggability
scheduledExecutorService = Executors.newSingleThreadScheduledExecutor(
new NamedThreadFactory("Dubbo-Nacos-Registry-Scheduler", true)
);
scheduledExecutorService.scheduleAtFixedRate(
() -> {
// ... existing logic ...
},
LOOKUP_INTERVAL,
LOOKUP_INTERVAL,
TimeUnit.SECONDS);
}
}
}
}

### Are you willing to submit a pull request to fix on your own?

- [x] Yes I am willing to submit a pull request on my own!

### Code of Conduct

- [x] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct)

Contributor guide

Open the contributing guide

Research direction

Locate org.apache.dubbo.registry.nacos.NacosRegistry and read scheduleServiceNamesLookup, focusing on the lazy scheduledExecutorService initialization and its concurrent callers. Validate the race with the project’s relevant Nacos registry tests or a focused concurrent reproduction; done means concurrent calls create only one scheduler without a thread leak.

Written by the indexing model from the issue text.

Assessment

Tech stack
java
Domain
backend, distributed-systems
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.