funky-eyes commented on code in PR #7349:
URL: https://github.com/apache/incubator-seata/pull/7349#discussion_r2095768760


##########
discovery/seata-discovery-etcd3/src/main/java/org/apache/seata/discovery/registry/etcd3/EtcdRegistryServiceImpl.java:
##########
@@ -401,20 +411,24 @@ public EtcdWatcher(String cluster, Watch.Listener 
listener) {
         @Override
         public void run() {
             Watch watchClient = getClient().getWatchClient();
-            WatchOption.Builder watchOptionBuilder = 
WatchOption.newBuilder().withPrefix(buildRegistryKeyPrefix(cluster));
+            WatchOption.Builder watchOptionBuilder =
+                    
WatchOption.newBuilder().withPrefix(buildRegistryKeyPrefix(cluster));
             Pair<Long /*revision*/, List<InetSocketAddress>> addressPair = 
clusterAddressMap.get(cluster);
             if (Objects.nonNull(addressPair)) {
                 // Maybe addressPair isn't newest now, but it's ok
                 watchOptionBuilder.withRevision(addressPair.getKey());
             }
-            this.watcher = watchClient.watch(buildRegistryKeyPrefix(cluster), 
watchOptionBuilder.build(), this.listener);
+            this.watcher =
+                    watchClient.watch(buildRegistryKeyPrefix(cluster), 
watchOptionBuilder.build(), this.listener);
         }
 
         /**
          * stop this task
          */
         public void stop() {
-            this.watcher.close();
+            if (this.watcher != null) {
+                this.watcher.close();
+            }

Review Comment:
   > Currently, the `watcher` is initialized not in the `constructor`, but 
within the `run` method of the `EtcdWatcher` class. In the test code, we used 
`thenAnswer` to return a `mockWatcher`, ensuring that the watcher is properly 
initialized in the run method. However, when running the test `10,000` times 
using `@RepeatedTest`, we encountered 7 failures.
   > 
   > All 7 failures were caused by a `NullPointerException` triggered when the 
`stop` method was called while the watcher was still null.
   > 
   > 
![image](https://private-user-images.githubusercontent.com/104314593/445156771-718007ab-8307-45a1-88e2-f97242382ca4.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NDc2NjMwNDIsIm5iZiI6MTc0NzY2Mjc0MiwicGF0aCI6Ii8xMDQzMTQ1OTMvNDQ1MTU2NzcxLTcxODAwN2FiLTgzMDctNDVhMS04OGUyLWY5NzI0MjM4MmNhNC5wbmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjUwNTE5JTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI1MDUxOVQxMzUyMjJaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT1mYWI5ZjZkMTE5NzQ0YmFhMTU2NDBiNTQ5NWMxZjVjZjI3Y2Y2OTViMjYyMWRiOGVjYTcwNjY1NWI3MTk1MzY0JlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCJ9.m_oUfSAYjStjxA475ucrWwnVye2WQsW11hTocSTGMy0)
   > 
   > It appears that, intermittently, the unsubscribe method is executed before 
the run method, leading to this issue.
   > 
   > * After adding a null check in the `stop` method, the issue has not 
occurred again.
   >   That's why i believe this piece of code is related to the NPE issue.
   
   I understand now, thank you for your explanation.



-- 
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: notifications-unsubscr...@seata.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@seata.apache.org
For additional commands, e-mail: notifications-h...@seata.apache.org

Reply via email to