Return-Path: X-Original-To: apmail-curator-commits-archive@minotaur.apache.org Delivered-To: apmail-curator-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id 27DB017D8C for ; Wed, 6 May 2015 04:56:00 +0000 (UTC) Received: (qmail 67741 invoked by uid 500); 6 May 2015 04:56:00 -0000 Delivered-To: apmail-curator-commits-archive@curator.apache.org Received: (qmail 67674 invoked by uid 500); 6 May 2015 04:56:00 -0000 Mailing-List: contact commits-help@curator.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@curator.apache.org Delivered-To: mailing list commits@curator.apache.org Received: (qmail 66774 invoked by uid 99); 6 May 2015 04:55:59 -0000 Received: from git1-us-west.apache.org (HELO git1-us-west.apache.org) (140.211.11.23) by apache.org (qpsmtpd/0.29) with ESMTP; Wed, 06 May 2015 04:55:59 +0000 Received: by git1-us-west.apache.org (ASF Mail Server at git1-us-west.apache.org, from userid 33) id 53B4AE35A7; Wed, 6 May 2015 04:55:59 +0000 (UTC) Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit From: randgalt@apache.org To: commits@curator.apache.org Date: Wed, 06 May 2015 04:56:24 -0000 Message-Id: In-Reply-To: <34422cdf5d34428891ea339c0ed20424@git.apache.org> References: <34422cdf5d34428891ea339c0ed20424@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [27/50] curator git commit: sync on holder for safety during multi-step operations sync on holder for safety during multi-step operations Project: http://git-wip-us.apache.org/repos/asf/curator/repo Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/c62b1137 Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/c62b1137 Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/c62b1137 Branch: refs/heads/CURATOR-160 Commit: c62b1137fa25104f2e24d65e467d0cfc769bd6e2 Parents: fa0c9da Author: randgalt Authored: Tue Apr 21 16:47:10 2015 -0500 Committer: randgalt Committed: Tue Apr 21 16:47:10 2015 -0500 ---------------------------------------------------------------------- .../discovery/details/ServiceDiscoveryImpl.java | 86 ++++++++++++-------- 1 file changed, 53 insertions(+), 33 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/curator/blob/c62b1137/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/ServiceDiscoveryImpl.java ---------------------------------------------------------------------- diff --git a/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/ServiceDiscoveryImpl.java b/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/ServiceDiscoveryImpl.java index 80b012e..ba18e42 100644 --- a/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/ServiceDiscoveryImpl.java +++ b/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/ServiceDiscoveryImpl.java @@ -58,6 +58,7 @@ import java.util.concurrent.atomic.AtomicReference; /** * A mechanism to register and query service instances using ZooKeeper */ +@SuppressWarnings("SynchronizationOnLocalVariableOrMethodParameter") public class ServiceDiscoveryImpl implements ServiceDiscovery { private final Logger log = LoggerFactory.getLogger(getClass()); @@ -207,16 +208,19 @@ public class ServiceDiscoveryImpl implements ServiceDiscovery { clean(); - Holder holder = getOrMakeHolder(service, null); - if ( holder.state.get() == State.UNREGISTERED ) + final Holder holder = getOrMakeHolder(service, null); + synchronized(holder) { - throw new Exception("Service has been unregistered: " + service); - } + if ( holder.state.get() == State.UNREGISTERED ) + { + throw new Exception("Service has been unregistered: " + service); + } - holder.service.set(service); - byte[] bytes = serializer.serialize(service); - String path = pathForInstance(service.getName(), service.getId()); - client.setData().forPath(path, bytes); + holder.service.set(service); + byte[] bytes = serializer.serialize(service); + String path = pathForInstance(service.getName(), service.getId()); + client.setData().forPath(path, bytes); + } } @VisibleForTesting @@ -455,17 +459,27 @@ public class ServiceDiscoveryImpl implements ServiceDiscovery @VisibleForTesting ServiceInstance getRegisteredService(String id) { - Holder holder = services.get(id); - return ((holder != null) && (holder.state.get() == State.REGISTERED)) ? holder.service.get() : null; + final Holder holder = services.get(id); + if ( holder != null ) + { + synchronized(holder) + { + return (holder.state.get() == State.REGISTERED) ? holder.service.get() : null; + } + } + return null; } private void reRegisterServices() throws Exception { - for ( Holder service : services.values() ) + for ( final Holder holder : services.values() ) { - if ( service.state.get() == State.REGISTERED ) + synchronized(holder) { - internalRegisterService(service.service.get()); + if ( holder.state.get() == State.REGISTERED ) + { + internalRegisterService(holder.service.get()); + } } } } @@ -529,39 +543,45 @@ public class ServiceDiscoveryImpl implements ServiceDiscovery Iterator> iterator = services.values().iterator(); while ( iterator.hasNext() ) { - Holder holder = iterator.next(); - if ( holder.state.get() == State.UNREGISTERED ) + final Holder holder = iterator.next(); + synchronized(holder) { - long elapsed = System.currentTimeMillis() - holder.stateChangeMs.get(); - if ( elapsed >= CLEAN_THRESHOLD_MS ) + if ( holder.state.get() == State.UNREGISTERED ) { - iterator.remove(); + long elapsed = System.currentTimeMillis() - holder.stateChangeMs.get(); + if ( elapsed >= CLEAN_THRESHOLD_MS ) + { + iterator.remove(); + } } } } } } - private void internalUnregisterService(Holder holder) throws Exception + private void internalUnregisterService(final Holder holder) throws Exception { if ( holder != null ) { - holder.setState(State.UNREGISTERED); - NodeCache cache = holder.cache.getAndSet(null); - if ( cache != null ) + synchronized(holder) { - CloseableUtils.closeQuietly(cache); - } + holder.setState(State.UNREGISTERED); + NodeCache cache = holder.cache.getAndSet(null); + if ( cache != null ) + { + CloseableUtils.closeQuietly(cache); + } - ServiceInstance service = holder.service.get(); - String path = pathForInstance(service.getName(), service.getId()); - try - { - client.delete().guaranteed().forPath(path); - } - catch ( KeeperException.NoNodeException ignore ) - { - // ignore + ServiceInstance service = holder.service.get(); + String path = pathForInstance(service.getName(), service.getId()); + try + { + client.delete().guaranteed().forPath(path); + } + catch ( KeeperException.NoNodeException ignore ) + { + // ignore + } } } }