ZookeeperRegistryDataCacheImpl may be not threadSafe

[OrezzerO]

The way we use ConcurrentHashMap may be not correct. For example:

 List<RpcProvider> currentProviderList = providers.get(rpcService);
        if (currentProviderList == null) {
            //todo 2019-07-05 11:23 线程安全问题
            providers.put(rpcService, providerList);
        } else {
            for (RpcProvider provider : providerList) {
                if (currentProviderList.contains(provider)) {
                    continue;
                }
                currentProviderList.add(provider);
            }
        }

Sometimes put() method may be invoked twice, it is not we expected.

[chpengzh]

All method are invoked by PathChildrenCacheListener.childEvent, which will be invoke by a single thread with event order. You can debug it or print a thread id to find that.

RegistryDataCache is not thread safe, yes, but it is used in a thread safe case.

[OrezzerO]

Got it. ZooKeeper watches are single threaded. https://cwiki.apache.org/confluence/display/CURATOR/TN1
So ,it is unnecessary to use ConcurrentHashMap?

[chpengzh]

Although it is written by one and read by others, it is better to be thread safe in my opinion. A better practice is using Map.compute or Map.computeIfAbsent rather than multi changes above.

See sofa-dashboard-client for more details.