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 769F817D95 for ; Wed, 6 May 2015 04:56:00 +0000 (UTC) Received: (qmail 68040 invoked by uid 500); 6 May 2015 04:56:00 -0000 Delivered-To: apmail-curator-commits-archive@curator.apache.org Received: (qmail 67959 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 66830 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 62B4BE3597; 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:29 -0000 Message-Id: <6fdd0cb2f04c41ad86c77080da98fdfa@git.apache.org> In-Reply-To: <34422cdf5d34428891ea339c0ed20424@git.apache.org> References: <34422cdf5d34428891ea339c0ed20424@git.apache.org> X-Mailer: ASF-Git Admin Mailer Subject: [32/50] curator git commit: Use sync instead of locks. It's simpler and clearer Use sync instead of locks. It's simpler and clearer Project: http://git-wip-us.apache.org/repos/asf/curator/repo Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/1a16f82b Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/1a16f82b Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/1a16f82b Branch: refs/heads/CURATOR-160 Commit: 1a16f82baae0360a2823db2cc811fbeb3d6e1392 Parents: f8b67dc Author: randgalt Authored: Mon Apr 27 14:51:46 2015 -0500 Committer: randgalt Committed: Mon Apr 27 14:51:46 2015 -0500 ---------------------------------------------------------------------- .../curator/x/discovery/details/Holder.java | 106 ++++--------------- .../discovery/details/ServiceDiscoveryImpl.java | 24 +---- 2 files changed, 26 insertions(+), 104 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/curator/blob/1a16f82b/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/Holder.java ---------------------------------------------------------------------- diff --git a/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/Holder.java b/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/Holder.java index cbc6236..d088f8d 100644 --- a/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/Holder.java +++ b/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/Holder.java @@ -2,8 +2,6 @@ package org.apache.curator.x.discovery.details; import org.apache.curator.framework.recipes.cache.NodeCache; import org.apache.curator.x.discovery.ServiceInstance; -import java.util.concurrent.locks.Lock; -import java.util.concurrent.locks.ReentrantLock; class Holder { @@ -18,7 +16,6 @@ class Holder private NodeCache cache; private State state; private long stateChangeMs; - private final ReentrantLock lock = new ReentrantLock(); Holder(ServiceInstance service, NodeCache nodeCache) { @@ -27,110 +24,49 @@ class Holder setState(State.NEW); } - ServiceInstance getService() + synchronized ServiceInstance getService() { - lock.lock(); - try - { - return service; - } - finally - { - lock.unlock(); - } + return service; } - ServiceInstance getServiceIfRegistered() + synchronized ServiceInstance getServiceIfRegistered() { - lock.lock(); - try - { - return (state == State.REGISTERED) ? service : null; - } - finally - { - lock.unlock(); - } + return (state == State.REGISTERED) ? service : null; } - void setService(ServiceInstance service) + synchronized void setService(ServiceInstance service) { - lock.lock(); - try - { - this.service = service; - } - finally - { - lock.unlock(); - } + this.service = service; } - NodeCache getAndClearCache() + synchronized NodeCache getAndClearCache() { - lock.lock(); - try - { - NodeCache localCache = cache; - cache = null; - return localCache; - } - finally - { - lock.unlock(); - } + NodeCache localCache = cache; + cache = null; + return localCache; } - boolean isRegistered() + synchronized boolean isRegistered() { - lock.lock(); - try - { - return state == State.REGISTERED; - } - finally - { - lock.unlock(); - } + return state == State.REGISTERED; } - boolean isLapsedUnregistered(int cleanThresholdMs) + synchronized boolean isLapsedUnregistered(int cleanThresholdMs) { - lock.lock(); - try + if ( state == State.UNREGISTERED ) { - if ( state == State.UNREGISTERED ) + long elapsed = System.currentTimeMillis() - stateChangeMs; + if ( elapsed >= cleanThresholdMs ) { - long elapsed = System.currentTimeMillis() - stateChangeMs; - if ( elapsed >= cleanThresholdMs ) - { - return true; - } + return true; } - return false; - } - finally - { - lock.unlock(); - } - } - - void setState(State state) - { - lock.lock(); - try - { - this.state = state; - stateChangeMs = System.currentTimeMillis(); - } - finally - { - lock.unlock(); } + return false; } - Lock getLock() + synchronized void setState(State state) { - return lock; + this.state = state; + stateChangeMs = System.currentTimeMillis(); } } http://git-wip-us.apache.org/repos/asf/curator/blob/1a16f82b/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 8e3e1f9..a35cd3a 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 @@ -59,6 +59,7 @@ import java.util.concurrent.atomic.AtomicLong; /** * A mechanism to register and query service instances using ZooKeeper */ +@SuppressWarnings("SynchronizationOnLocalVariableOrMethodParameter") public class ServiceDiscoveryImpl implements ServiceDiscovery { private final Logger log = LoggerFactory.getLogger(getClass()); @@ -181,9 +182,8 @@ public class ServiceDiscoveryImpl implements ServiceDiscovery { clean(); - Holder holder = getOrMakeHolder(service, null); - holder.getLock().lock(); - try + final Holder holder = getOrMakeHolder(service, null); + synchronized(holder) { if ( !holder.isRegistered() ) { @@ -195,10 +195,6 @@ public class ServiceDiscoveryImpl implements ServiceDiscovery String path = pathForInstance(service.getName(), service.getId()); client.setData().forPath(path, bytes); } - finally - { - holder.getLock().unlock(); - } } @VisibleForTesting @@ -459,18 +455,13 @@ public class ServiceDiscoveryImpl implements ServiceDiscovery { for ( final Holder holder : services.values() ) { - holder.getLock().lock(); - try + synchronized(holder) { if ( holder.isRegistered() ) { internalRegisterService(holder.getService()); } } - finally - { - holder.getLock().unlock(); - } } } @@ -544,8 +535,7 @@ public class ServiceDiscoveryImpl implements ServiceDiscovery { if ( holder != null ) { - holder.getLock().lock(); - try + synchronized(holder) { holder.setState(Holder.State.UNREGISTERED); NodeCache cache = holder.getAndClearCache(); @@ -565,10 +555,6 @@ public class ServiceDiscoveryImpl implements ServiceDiscovery // ignore } } - finally - { - holder.getLock().unlock(); - } } } }