QPID-6208: [Java Broker] Avoid possibility of race condition when setting the permitted node list

* Environment thread and broker configuration thread could race, which had the potential to lead to the
  permittedNodeList being removed.

git-svn-id: https://svn.apache.org/repos/asf/qpid/trunk@1637845 13f79535-47bb-0310-9956-ffa450edef68

2 files changed, 10 insertions, 14 deletions

diff --git a/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/replication/ReplicatedEnvironmentFacade.java b/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/replication/ReplicatedEnvironmentFacade.java
index 675707d4ec..9d3751caec 100644
--- a/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/replication/ReplicatedEnvironmentFacade.java
+++ b/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/store/berkeleydb/replication/ReplicatedEnvironmentFacade.java
@@ -963,8 +963,6 @@ public class ReplicatedEnvironmentFacade implements EnvironmentFacade, StateChan
 
         createEnvironment(false);
 
-        registerAppStateMonitorIfPermittedNodesSpecified();
-
         if (stateChangeListener != null)
         {
             _environment.setStateChangeListener(this);
@@ -1279,7 +1277,10 @@ public class ReplicatedEnvironmentFacade implements EnvironmentFacade, StateChan
         if (permittedNodes != null)
         {
             _permittedNodes.addAll(permittedNodes);
-            registerAppStateMonitorIfPermittedNodesSpecified();
+            // We register an app state monitor containing with permitted node list on
+            // all nodes so that any node can be used as the helper when adding more nodes
+            // to the group
+            registerAppStateMonitorIfPermittedNodesSpecified(_permittedNodes);
 
             ReplicationGroupListener listener = _replicationGroupListener.get();
             int count = 0;
@@ -1298,11 +1299,6 @@ public class ReplicatedEnvironmentFacade implements EnvironmentFacade, StateChan
         }
     }
 
-    Set<String> getPermittedNodes()
-    {
-        return Collections.unmodifiableSet(_permittedNodes);
-    }
-
     static NodeState getRemoteNodeState(String groupName, ReplicationNode repNode, int dbPingSocketTimeout) throws IOException, ServiceConnectFailedException
     {
         if (repNode == null)
@@ -1379,11 +1375,11 @@ public class ReplicatedEnvironmentFacade implements EnvironmentFacade, StateChan
         return permittedNodes;
     }
 
-    private void registerAppStateMonitorIfPermittedNodesSpecified()
+    private void registerAppStateMonitorIfPermittedNodesSpecified(final Set<String> permittedNodes)
     {
-        if (!_permittedNodes.isEmpty())
+        if (!permittedNodes.isEmpty())
         {
-            byte[] data = permittedNodeListToBytes(_permittedNodes);
+            byte[] data = permittedNodeListToBytes(permittedNodes);
             _environment.registerAppStateMonitor(new EnvironmentStateHolder(data));
         }
     }
diff --git a/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHAVirtualHostNodeImpl.java b/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHAVirtualHostNodeImpl.java
index 41bba263a9..7d877a8b00 100644
--- a/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHAVirtualHostNodeImpl.java
+++ b/qpid/java/bdbstore/src/main/java/org/apache/qpid/server/virtualhostnode/berkeleydb/BDBHAVirtualHostNodeImpl.java
@@ -1036,8 +1036,8 @@ public class BDBHAVirtualHostNodeImpl extends AbstractVirtualHostNode<BDBHAVirtu
                                     setAttribute(PERMITTED_NODES, _permittedNodes, new ArrayList<String>(permittedNodes));
                                 } else
                                 {
-                                    LOGGER.warn("Cannot change permitted nodes from Master as existing master node '" + remoteNode.getName()
-                                            + "' (" + remoteNode.getAddress() + ") is not in list of trusted nodes " + _permittedNodes);
+                                    LOGGER.warn("Cannot accept the new permitted node list from the master as the master '" + remoteNode.getName()
+                                            + "' (" + remoteNode.getAddress() + ") was not in previous permitted list " + _permittedNodes);
                                 }
                             }
                         }
@@ -1045,7 +1045,7 @@ public class BDBHAVirtualHostNodeImpl extends AbstractVirtualHostNode<BDBHAVirtu
                         {
                             if (LOGGER.isDebugEnabled())
                             {
-                                LOGGER.debug(String.format("Application state returned by JE was 'null' so skipping permitted node handling: s%", nodeState));
+                                LOGGER.debug(String.format("Application state returned by JE was 'null' so skipping permitted node handling: %s", nodeState));
                             }
                         }
                     }