Merge pull request #9435 from vespa-engine/balder/ensure-we-do-not-drop-close-destroy-on-exception-or-create-some-that-we-do-not-destroy

- Use double checked locking to ensure that we do not create Policies…

3 files changed, 38 insertions, 13 deletions

diff --git a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/StoragePolicy.java b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/StoragePolicy.java
index 918bd193d89..552f47ec5a6 100644
--- a/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/StoragePolicy.java
+++ b/documentapi/src/main/java/com/yahoo/documentapi/messagebus/protocol/StoragePolicy.java
@@ -349,15 +349,24 @@ public class StoragePolicy extends SlobrokPolicy {
         private final int maxOldClusterVersionBeforeSendingRandom; // Reset cluster version protection
 
         DistributorSelectionLogic(Parameters params, SlobrokPolicy policy) {
-            this.hostFetcher = params.createHostFetcher(policy, params.getRequiredUpPercentageToSendToKnownGoodNodes());
-            this.distribution = params.createDistribution(policy);
-            persistentFailureChecker = new InstabilityChecker(params.getAttemptRandomOnFailuresLimit());
-            maxOldClusterVersionBeforeSendingRandom = params.maxOldClusterStatesSeenBeforeThrowingCachedState();
+            try {
+                hostFetcher = params.createHostFetcher(policy, params.getRequiredUpPercentageToSendToKnownGoodNodes());
+                distribution = params.createDistribution(policy);
+                persistentFailureChecker = new InstabilityChecker(params.getAttemptRandomOnFailuresLimit());
+                maxOldClusterVersionBeforeSendingRandom = params.maxOldClusterStatesSeenBeforeThrowingCachedState();
+            } catch (Throwable e) {
+                destroy();
+                throw e;
+            }
         }
 
         public void destroy() {
-            hostFetcher.close();
-            distribution.close();
+            if (hostFetcher != null) {
+                hostFetcher.close();
+            }
+            if (distribution != null) {
+                distribution.close();
+            }
         }
 
         String getTargetSpec(RoutingContext context, BucketId bucketId) {
diff --git a/messagebus/src/main/java/com/yahoo/messagebus/ProtocolRepository.java b/messagebus/src/main/java/com/yahoo/messagebus/ProtocolRepository.java
index ad26475bf74..102e0be923d 100755
--- a/messagebus/src/main/java/com/yahoo/messagebus/ProtocolRepository.java
+++ b/messagebus/src/main/java/com/yahoo/messagebus/ProtocolRepository.java
@@ -28,7 +28,7 @@ public class ProtocolRepository {
      */
     public void putProtocol(Protocol protocol) {
         if (protocols.put(protocol.getName(), protocol) != null) {
-            routingPolicyCache.clear();
+            clearPolicyCache();
         }
     }
 
@@ -72,6 +72,10 @@ public class ProtocolRepository {
             return ret;
         }
         synchronized (this) {
+            ret = routingPolicyCache.get(cacheKey);
+            if (ret != null) {
+                return ret;
+            }
             Protocol protocol = getProtocol(protocolName);
             if (protocol == null) {
                 log.log(LogLevel.ERROR, "Protocol '" + protocolName + "' not supported.");
@@ -81,6 +85,9 @@ public class ProtocolRepository {
                 ret = protocol.createPolicy(policyName, policyParam);
             } catch (RuntimeException e) {
                 log.log(LogLevel.ERROR, "Protcol '" + protocolName + "' threw an exception: " + e.getMessage(), e);
+                if (ret != null) {
+                    ret.destroy();
+                }
                 return null;
             }
             if (ret == null) {
diff --git a/vdslib/src/main/java/com/yahoo/vdslib/distribution/Distribution.java b/vdslib/src/main/java/com/yahoo/vdslib/distribution/Distribution.java
index 0063011e41c..47a09ab0873 100644
--- a/vdslib/src/main/java/com/yahoo/vdslib/distribution/Distribution.java
+++ b/vdslib/src/main/java/com/yahoo/vdslib/distribution/Distribution.java
@@ -108,8 +108,13 @@ public class Distribution {
             distributionBitMasks[i] = mask;
             mask = (mask << 1) | 1;
         }
-        configSub = new ConfigSubscriber();
-        configSub.subscribe(configSubscriber, StorDistributionConfig.class, configId);
+        try {
+            configSub = new ConfigSubscriber();
+            configSub.subscribe(configSubscriber, StorDistributionConfig.class, configId);
+        } catch (Throwable e) {
+            close();
+            throw e;
+        }
     }
 
     public Distribution(StorDistributionConfig config) {
@@ -122,7 +127,11 @@ public class Distribution {
     }
 
     public void close() {
-        if (configSub!=null) configSub.close();
+        if (configSub!=null) {
+            configSub.close();
+            configSub = null;
+        }
+        configSubscriber = null;
     }
 
     private int getGroupSeed(BucketId bucket, ClusterState state, Group group) {
@@ -146,7 +155,7 @@ public class Distribution {
         return seed;
     }
 
-    private class ScoredGroup implements Comparable<ScoredGroup> {
+    private static class ScoredGroup implements Comparable<ScoredGroup> {
         Group group;
         double score;
 
@@ -158,7 +167,7 @@ public class Distribution {
             return Double.valueOf(o.score).compareTo(score);
         }
     }
-    private class ScoredNode {
+    private static class ScoredNode {
         int index;
         int reliability;
         double score;
@@ -205,7 +214,7 @@ public class Distribution {
         }
         return getIdealDistributorGroup(bucket, clusterState, results.first().group, redundancyArray[0]);
     }
-    private class ResultGroup implements Comparable<ResultGroup> {
+    private static class ResultGroup implements Comparable<ResultGroup> {
         Group group;
         int redundancy;