[ https://issues.apache.org/jira/browse/CURATOR-164?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14505317#comment-14505317 ]
ASF GitHub Bot commented on CURATOR-164: ---------------------------------------- Github user Randgalt commented on a diff in the pull request: https://github.com/apache/curator/pull/72#discussion_r28801169 --- Diff: curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/ServiceDiscoveryImpl.java --- @@ -228,15 +234,18 @@ protected void internalRegisterService(ServiceInstance<T> service) throws Ex public void unregisterService(ServiceInstance<T> service) throws Exception { String path = pathForInstance(service.getName(), service.getId()); - try - { - client.delete().forPath(path); - } - catch ( KeeperException.NoNodeException ignore ) - { - // ignore + synchronized(service) { --- End diff -- I don't think the sync adds anything. The services are usually copies. > curator-x-discovery: unregisterService is not guaranteed to remove the > service, due to reconnectListener concurrency issue > -------------------------------------------------------------------------------------------------------------------------- > > Key: CURATOR-164 > URL: https://issues.apache.org/jira/browse/CURATOR-164 > Project: Apache Curator > Issue Type: Bug > Components: Framework > Affects Versions: 2.7.0 > Reporter: Rasmus Berg Palm > Assignee: Jordan Zimmerman > Priority: Critical > Fix For: 2.8.0 > > > In ServiceDiscoveryImpl: > When unregistering a service, the reconnect listener might fire while > deleting the path. > This can cause a condition where the delete finishes successfully, the > service is removed from services, and then the reRegisterServices completes > successfully and the service is added back in ZK and in services, end result > being that the service was not removed, even though unregisterService did not > throw any exceptions. > Essentially the use of the internal 'services' cache makes for a nightmare of > concurrency issues. I put this as critical as the library it's really not > usable IMO. -- This message was sent by Atlassian JIRA (v6.3.4#6332)