Do not expose Timer from ServerSessionManager

The timer has a strict lifecycle, make sure it cannot be leaked.

Change-Id: I63356ce6f706bf3864cca4cd8a953bf758449ba3
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>

diff --git a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractTopologySessionListener.java b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractTopologySessionListener.java
index c976b9b4ffbfa2c7f747b1d4155eb49bb49d94e6..3d9d1438aba761039dcf4ceeef7ce8e0153ae9c9 100644 (file)
--- a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractTopologySessionListener.java
+++ b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractTopologySessionListener.java
@@ -15,7 +15,6 @@ 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 io.netty.util.Timeout;
 import io.netty.util.concurrent.Future;
 import java.net.InetAddress;
 import java.util.ArrayList;
@@ -27,7 +26,6 @@ import java.util.Map.Entry;
 import java.util.Objects;
 import java.util.Optional;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.TimeUnit;
 import java.util.concurrent.atomic.AtomicBoolean;
 import org.checkerframework.checker.lock.qual.GuardedBy;
 import org.checkerframework.checker.lock.qual.Holding;
@@ -415,17 +413,10 @@ public abstract class AbstractTopologySessionListener implements TopologySession
         final var sendFuture = session.sendMessage(message);
         listenerState.updateStatefulSentMsg(message);
 
-        final short rpcTimeout = serverSessionManager.getRpcTimeout();
-        LOG.trace("RPC response timeout value is {} seconds", rpcTimeout);
-
-        final Timeout timeout;
-        if (rpcTimeout > 0) {
-            // Note: the timeout is held back by us holding the 'this' monitor, which timeoutExpired re-acquires
-            timeout = serverSessionManager.timer().newTimeout(ignored -> timeoutExpired(requestId),
-                rpcTimeout, TimeUnit.SECONDS);
+        // Note: the timeout is held back by us holding the 'this' monitor, which timeoutExpired re-acquires
+        final var timeout = serverSessionManager.newRpcTimeout(this::timeoutExpired, requestId);
+        if (timeout != null) {
             LOG.trace("Set up response timeout handler for request {}", requestId);
-        } else {
-            timeout = null;
         }
 
         final PCEPRequest req = new PCEPRequest(metadata, timeout);

diff --git a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/ServerSessionManager.java b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/ServerSessionManager.java
index 20e50352558820275d12cff0c61466fccd2e9dcc..310f574da439aa8e97499decff7c4fa1f1f3b6f3 100644 (file)
--- a/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/ServerSessionManager.java
+++ b/pcep/topology/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/ServerSessionManager.java
@@ -16,7 +16,7 @@ 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.HashedWheelTimer;
-import io.netty.util.Timer;
+import io.netty.util.Timeout;
 import java.net.InetAddress;
 import java.util.HashMap;
 import java.util.List;
@@ -25,12 +25,14 @@ 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;
 import org.opendaylight.bgpcep.pcep.server.PceServerProvider;
 import org.opendaylight.mdsal.binding.api.WriteTransaction;
 import org.opendaylight.mdsal.common.api.CommitInfo;
 import org.opendaylight.mdsal.common.api.LogicalDatastoreType;
 import org.opendaylight.protocol.pcep.PCEPSessionListenerFactory;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.graph.rev220720.graph.topology.GraphKey;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.pcep.ietf.stateful.rev200720.SrpIdNumber;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.pcep.stats.rev171113.PcepSessionState;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.topology.pcep.rev220730.AddLspArgs;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.topology.pcep.rev220730.EnsureLspOperationalInput;
@@ -58,6 +60,11 @@ import org.slf4j.LoggerFactory;
 
 // Non-final for testing
 class ServerSessionManager implements PCEPSessionListenerFactory, TopologySessionRPCs {
+    @FunctionalInterface
+    interface RpcTimeout {
+        void run(SrpIdNumber requestId);
+    }
+
     private static final Logger LOG = LoggerFactory.getLogger(ServerSessionManager.class);
     private static final long DEFAULT_HOLD_STATE_NANOS = TimeUnit.MINUTES.toNanos(5);
 
@@ -273,12 +280,10 @@ class ServerSessionManager implements PCEPSessionListenerFactory, TopologySessio
             .buildFuture();
     }
 
-    final @NonNull Timer timer() {
-        return timer;
-    }
-
-    final short getRpcTimeout() {
-        return rpcTimeout;
+    final @Nullable Timeout newRpcTimeout(final RpcTimeout task, final SrpIdNumber requestId) {
+        final short localTimeout = rpcTimeout;
+        return localTimeout <= 0 ? null
+            : timer.newTimeout(ignored -> task.run(requestId), localTimeout, TimeUnit.SECONDS);
     }
 
     final void setRpcTimeout(final short rpcTimeout) {