Fix concurrent access of node permissioning allow list (#7920)

Signed-off-by: Bhanu Pulluri <bhanu.pulluri@kaleido.io>
Co-authored-by: Bhanu Pulluri <bhanu.pulluri@kaleido.io>
Co-authored-by: Sally MacFarlane <macfarla.github@gmail.com>

ethereum/permissioning/src/main/java/org/hyperledger/besu/ethereum/permissioning/NodeLocalConfigPermissioningController.java

@@ -32,6 +32,7 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Objects;
 import java.util.Optional;
+import java.util.concurrent.CopyOnWriteArrayList;
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
 
@@ -48,7 +49,7 @@ public class NodeLocalConfigPermissioningController implements NodeConnectionPer
   private LocalPermissioningConfiguration configuration;
   private final List<EnodeURL> fixedNodes;
   private final Bytes localNodeId;
-  private final List<EnodeURL> nodesAllowlist = new ArrayList<>();
+  private final List<EnodeURL> nodesAllowlist = new CopyOnWriteArrayList<>();
   private final AllowlistPersistor allowlistPersistor;
   private final Subscribers<Consumer<NodeAllowlistUpdatedEvent>> nodeAllowlistUpdatedObservers =
       Subscribers.create();

ethereum/permissioning/src/test/java/org/hyperledger/besu/ethereum/permissioning/NodeLocalConfigPermissioningControllerTest.java

@@ -46,6 +46,7 @@ import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.function.Consumer;
 
 import com.google.common.collect.Lists;
@@ -316,6 +317,56 @@ public class NodeLocalConfigPermissioningControllerTest {
         .isTrue();
   }
 
+  @Test
+  public void whenCallingIsPermittedAndRemovingEntryInAnotherThreadShouldNotThrowException()
+      throws InterruptedException {
+    // Add a node to the allowlist
+    controller.addNodes(Lists.newArrayList(enode1));
+
+    // Atomic flag to detect exceptions
+    AtomicBoolean exceptionOccurred = new AtomicBoolean(false);
+
+    // Create a thread to call isPermitted
+    Thread isPermittedThread =
+        new Thread(
+            () -> {
+              try {
+                for (int i = 0; i < 1000; i++) {
+                  controller.isPermitted(enode1);
+                }
+              } catch (Exception e) {
+                exceptionOccurred.set(true);
+                e.printStackTrace();
+              }
+            });
+
+    // Create a thread to modify the allowlist
+    Thread modifyAllowlistThread =
+        new Thread(
+            () -> {
+              try {
+                for (int i = 0; i < 1000; i++) {
+                  controller.removeNodes(Lists.newArrayList(enode1));
+                  controller.addNodes(Lists.newArrayList(enode1));
+                }
+              } catch (Exception e) {
+                exceptionOccurred.set(true);
+                e.printStackTrace();
+              }
+            });
+
+    // Start both threads
+    isPermittedThread.start();
+    modifyAllowlistThread.start();
+
+    // Wait for both threads to complete
+    isPermittedThread.join();
+    modifyAllowlistThread.join();
+
+    // Assert no exceptions were thrown
+    assert (!exceptionOccurred.get());
+  }
+
   @Test
   public void stateShouldRevertIfAllowlistPersistFails()
       throws IOException, AllowlistFileSyncException {