Cleanup ServerSessionManager 71/101971/3
authorRobert Varga <robert.varga@pantheon.tech>
Fri, 5 Aug 2022 08:51:29 +0000 (10:51 +0200)
committerRobert Varga <robert.varga@pantheon.tech>
Fri, 5 Aug 2022 09:03:36 +0000 (11:03 +0200)
Make sure invariants are packed together and eliminate use of
AtomicBoolean.

Change-Id: Ib22f0336d6644794442364b73cb64778b879aea6
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/ServerSessionManager.java
pcep/topology/topology-provider/src/test/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractPCEPSessionTest.java

index 949ae033adc7c4094ac653900e2b2cee08e4f0a6..502488774ef8736f300a0cdedd21b1a757e84570 100644 (file)
@@ -9,19 +9,19 @@ package org.opendaylight.bgpcep.pcep.topology.provider;
 
 import static java.util.Objects.requireNonNull;
 
-import com.google.common.annotations.VisibleForTesting;
 import com.google.common.util.concurrent.FluentFuture;
 import com.google.common.util.concurrent.FutureCallback;
 import com.google.common.util.concurrent.ListenableFuture;
 import com.google.common.util.concurrent.MoreExecutors;
 import com.google.common.util.concurrent.SettableFuture;
 import io.netty.util.Timeout;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.VarHandle;
 import java.net.InetAddress;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicBoolean;
 import org.checkerframework.checker.lock.qual.GuardedBy;
 import org.eclipse.jdt.annotation.NonNull;
 import org.eclipse.jdt.annotation.Nullable;
@@ -63,25 +63,32 @@ class ServerSessionManager implements PCEPSessionListenerFactory, TopologySessio
 
     private static final Logger LOG = LoggerFactory.getLogger(ServerSessionManager.class);
     private static final long DEFAULT_HOLD_STATE_NANOS = TimeUnit.MINUTES.toNanos(5);
+    private static final VarHandle CLOSED;
+
+    static {
+        try {
+            CLOSED = MethodHandles.lookup().findVarHandle(ServerSessionManager.class, "closed", boolean.class);
+        } catch (NoSuchFieldException | IllegalAccessException e) {
+            throw new ExceptionInInitializerError(e);
+        }
+    }
 
     private final @NonNull KeyedInstanceIdentifier<Topology, TopologyKey> topology;
     private final @NonNull PCEPTopologyProviderDependencies dependencies;
+    private final @NonNull GraphKey graphKey;
 
-    @VisibleForTesting
-    final AtomicBoolean isClosed = new AtomicBoolean(false);
     @GuardedBy("this")
     private final Map<NodeId, TopologySessionListener> nodes = new HashMap<>();
     @GuardedBy("this")
     private final Map<NodeId, TopologyNodeState> state = new HashMap<>();
 
     private volatile short rpcTimeout;
+    private volatile boolean closed;
 
-    private final GraphKey graphKey;
-
-    ServerSessionManager(final KeyedInstanceIdentifier<Topology, TopologyKey> instanceIdentifier,
+    ServerSessionManager(final KeyedInstanceIdentifier<Topology, TopologyKey> topology,
             final PCEPTopologyProviderDependencies dependencies, final short rpcTimeout, final GraphKey graphKey) {
         this.dependencies = requireNonNull(dependencies);
-        topology = requireNonNull(instanceIdentifier);
+        this.topology = requireNonNull(topology);
         this.rpcTimeout = rpcTimeout;
         this.graphKey = requireNonNull(graphKey);
     }
@@ -103,14 +110,14 @@ class ServerSessionManager implements PCEPSessionListenerFactory, TopologySessio
             @Override
             public void onSuccess(final CommitInfo result) {
                 LOG.info("PCEP Topology {} created successfully.", topologyId());
-                isClosed.set(false);
+                closed = false;
                 future.set(Boolean.TRUE);
             }
 
             @Override
             public void onFailure(final Throwable failure) {
                 LOG.error("Failed to create PCEP Topology {}.", topologyId(), failure);
-                isClosed.set(true);
+                closed = true;
                 future.set(Boolean.FALSE);
             }
         }, MoreExecutors.directExecutor());
@@ -123,8 +130,12 @@ class ServerSessionManager implements PCEPSessionListenerFactory, TopologySessio
         return future;
     }
 
+    final boolean isClosed() {
+        return closed;
+    }
+
     final synchronized FluentFuture<? extends CommitInfo> stop() {
-        if (isClosed.getAndSet(true)) {
+        if (!CLOSED.compareAndSet(this, false, true)) {
             LOG.error("Session Manager has already been closed.");
             return CommitInfo.emptyFluentFuture();
         }
@@ -166,7 +177,7 @@ class ServerSessionManager implements PCEPSessionListenerFactory, TopologySessio
 
     final synchronized void releaseNodeState(final TopologyNodeState nodeState, final InetAddress peerAddress,
             final boolean persistNode) {
-        if (isClosed.get()) {
+        if (isClosed()) {
             LOG.error("Session Manager has already been closed.");
             return;
         }
@@ -182,7 +193,7 @@ class ServerSessionManager implements PCEPSessionListenerFactory, TopologySessio
     final synchronized TopologyNodeState takeNodeState(final InetAddress address,
             final TopologySessionListener sessionListener, final boolean retrieveNode) {
         final NodeId id = createNodeId(address);
-        if (isClosed.get()) {
+        if (isClosed()) {
             LOG.error("Server Session Manager is closed. Unable to create topology node {} with listener {}", id,
                 sessionListener);
             return null;
index 24e39b3d5de531a70c4a1c773a9d4a67ef2887df..6223c9ed9d64092e33c3dec2c758e235246f806c 100644 (file)
@@ -151,7 +151,7 @@ public abstract class AbstractPCEPSessionTest extends AbstractConcurrentDataBrok
 
     void startSessionManager() throws Exception {
         assertTrue(manager.start().get());
-        assertFalse(manager.isClosed.get());
+        assertFalse(manager.isClosed());
     }
 
     void stopSessionManager() {