curator-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From cammcken...@apache.org
Subject [1/2] git commit: CURATOR-131 use iterator.remove instead of foreach
Date Mon, 11 Aug 2014 06:31:44 GMT
Repository: curator
Updated Branches:
  refs/heads/master 6bd0cc3e9 -> d2c37d0ce


CURATOR-131 use iterator.remove instead of foreach

When iterating over a collection, if we try to remove elements that
can lead to undefined behaviour. We should use an iterator and its
remove method to do this safely.


Project: http://git-wip-us.apache.org/repos/asf/curator/repo
Commit: http://git-wip-us.apache.org/repos/asf/curator/commit/e38d1ced
Tree: http://git-wip-us.apache.org/repos/asf/curator/tree/e38d1ced
Diff: http://git-wip-us.apache.org/repos/asf/curator/diff/e38d1ced

Branch: refs/heads/master
Commit: e38d1ceda492e097ad9c94bb854838f2b1f7caa9
Parents: f5767c8
Author: Mike Drob <mdrob@cloudera.com>
Authored: Thu Jul 31 16:24:40 2014 -0500
Committer: Mike Drob <mdrob@cloudera.com>
Committed: Thu Jul 31 16:24:40 2014 -0500

----------------------------------------------------------------------
 .../discovery/details/DownInstanceManager.java  | 11 ++++++---
 .../discovery/details/ServiceDiscoveryImpl.java | 26 +++++++++++++++++---
 2 files changed, 31 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/curator/blob/e38d1ced/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/DownInstanceManager.java
----------------------------------------------------------------------
diff --git a/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/DownInstanceManager.java
b/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/DownInstanceManager.java
index cda0968..5b89566 100644
--- a/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/DownInstanceManager.java
+++ b/curator-x-discovery/src/main/java/org/apache/curator/x/discovery/details/DownInstanceManager.java
@@ -19,10 +19,13 @@
 package org.apache.curator.x.discovery.details;
 
 import com.google.common.collect.Maps;
+
 import org.apache.curator.x.discovery.DownInstancePolicy;
 import org.apache.curator.x.discovery.InstanceFilter;
 import org.apache.curator.x.discovery.ServiceInstance;
-import java.util.Map;
+
+import java.util.Iterator;
+import java.util.Map.Entry;
 import java.util.concurrent.ConcurrentMap;
 import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicLong;
@@ -77,12 +80,14 @@ class DownInstanceManager<T> implements InstanceFilter<T>
             return;
         }
 
-        for ( Map.Entry<ServiceInstance<?>, Status> entry : statuses.entrySet()
)
+        Iterator<Entry<ServiceInstance<?>, Status>> it = statuses.entrySet().iterator();
+        while ( it.hasNext() )
         {
+            Entry<ServiceInstance<?>, Status> entry = it.next();
             long elapsedMs = System.currentTimeMillis() - entry.getValue().startMs;
             if ( elapsedMs >= downInstancePolicy.getTimeoutMs() )
             {
-                statuses.remove(entry.getKey());
+                it.remove();
             }
         }
     }

http://git-wip-us.apache.org/repos/asf/curator/blob/e38d1ced/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 3924b40..ad6ce89 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
@@ -24,6 +24,7 @@ import com.google.common.collect.ImmutableList;
 import com.google.common.collect.Lists;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
+
 import org.apache.curator.utils.CloseableUtils;
 import org.apache.curator.framework.CuratorFramework;
 import org.apache.curator.framework.state.ConnectionState;
@@ -43,9 +44,11 @@ import org.apache.zookeeper.KeeperException;
 import org.apache.zookeeper.Watcher;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
+
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
 import java.util.Collection;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
 
@@ -122,18 +125,35 @@ public class ServiceDiscoveryImpl<T> implements ServiceDiscovery<T>
             CloseableUtils.closeQuietly(provider);
         }
 
-        for ( ServiceInstance<T> service : services.values() )
+        Iterator<ServiceInstance<T>> it = services.values().iterator();
+        while ( it.hasNext() )
         {
+            // Should not use unregisterService because of potential ConcurrentModificationException
+            // so we in-line the bulk of the method here
+            ServiceInstance<T> service = it.next();
+            String path = pathForInstance(service.getName(), service.getId());
+            boolean doRemove = true;
+            
             try
             {
-                unregisterService(service);
+                client.delete().forPath(path);
+            }
+            catch ( KeeperException.NoNodeException ignore )
+            {
+                // ignore
             }
             catch ( Exception e )
             {
+                doRemove = false;
                 log.error("Could not unregister instance: " + service.getName(), e);
             }
+            
+            if ( doRemove )
+            {
+                it.remove();
+            }
         }
-
+        
         client.getConnectionStateListenable().removeListener(connectionStateListener);
     }
 


Mime
View raw message