QiuYucheng2003 opened a new issue, #15886:
URL: https://github.com/apache/dubbo/issues/15886

   ### 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)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to