curator-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From randg...@apache.org
Subject [09/12] curator git commit: Use sync instead of locks. It's simpler and clearer
Date Mon, 27 Apr 2015 22:10:01 GMT
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/master
Commit: 1a16f82baae0360a2823db2cc811fbeb3d6e1392
Parents: f8b67dc
Author: randgalt <randgalt@apache.org>
Authored: Mon Apr 27 14:51:46 2015 -0500
Committer: randgalt <randgalt@apache.org>
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<T>
 {
@@ -18,7 +16,6 @@ class Holder<T>
     private NodeCache cache;
     private State state;
     private long stateChangeMs;
-    private final ReentrantLock lock = new ReentrantLock();
 
     Holder(ServiceInstance<T> service, NodeCache nodeCache)
     {
@@ -27,110 +24,49 @@ class Holder<T>
         setState(State.NEW);
     }
 
-    ServiceInstance<T> getService()
+    synchronized ServiceInstance<T> getService()
     {
-        lock.lock();
-        try
-        {
-            return service;
-        }
-        finally
-        {
-            lock.unlock();
-        }
+        return service;
     }
 
-    ServiceInstance<T> getServiceIfRegistered()
+    synchronized ServiceInstance<T> getServiceIfRegistered()
     {
-        lock.lock();
-        try
-        {
-            return (state == State.REGISTERED) ? service : null;
-        }
-        finally
-        {
-            lock.unlock();
-        }
+        return (state == State.REGISTERED) ? service : null;
     }
 
-    void setService(ServiceInstance<T> service)
+    synchronized void setService(ServiceInstance<T> 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<T> implements ServiceDiscovery<T>
 {
     private final Logger log = LoggerFactory.getLogger(getClass());
@@ -181,9 +182,8 @@ public class ServiceDiscoveryImpl<T> implements ServiceDiscovery<T>
     {
         clean();
 
-        Holder<T> holder = getOrMakeHolder(service, null);
-        holder.getLock().lock();
-        try
+        final Holder<T> holder = getOrMakeHolder(service, null);
+        synchronized(holder)
         {
             if ( !holder.isRegistered() )
             {
@@ -195,10 +195,6 @@ public class ServiceDiscoveryImpl<T> implements ServiceDiscovery<T>
             String path = pathForInstance(service.getName(), service.getId());
             client.setData().forPath(path, bytes);
         }
-        finally
-        {
-            holder.getLock().unlock();
-        }
     }
 
     @VisibleForTesting
@@ -459,18 +455,13 @@ public class ServiceDiscoveryImpl<T> implements ServiceDiscovery<T>
     {
         for ( final Holder<T> 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<T> implements ServiceDiscovery<T>
     {
         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<T> implements ServiceDiscovery<T>
                     // ignore
                 }
             }
-            finally
-            {
-                holder.getLock().unlock();
-            }
         }
     }
 }


Mime
View raw message