Read-only transactions were not being closed

We have failed to close read-only transactions, which can lead to resource
starvation.

Change-Id: I62dbf6b6409a65f279b01c3bb5b2238ffa6febc8
Signed-off-by: Robert Varga <rovarga@cisco.com>
(cherry picked from commit 16cdb27ad01eb27950aba2c6fd5e4b03881a623a)

diff --git a/pcep/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/TopologyNodeState.java b/pcep/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/TopologyNodeState.java
index 0415338a8a276eff720e2a5c41f8a4814f5dbbec..d75908706a5f52567b44a597796373b185a03c76 100644 (file)
--- a/pcep/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/TopologyNodeState.java
+++ b/pcep/topology-provider/src/main/java/org/opendaylight/bgpcep/pcep/topology/provider/TopologyNodeState.java
@@ -12,17 +12,14 @@ import com.google.common.base.Preconditions;
 import com.google.common.util.concurrent.FutureCallback;
 import com.google.common.util.concurrent.Futures;
 import com.google.common.util.concurrent.ListenableFuture;
-
 import java.util.Collection;
 import java.util.HashMap;
 import java.util.Iterator;
 import java.util.Map;
-
 import javax.annotation.concurrent.ThreadSafe;
-
 import org.opendaylight.controller.md.sal.binding.api.BindingTransactionChain;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
-import org.opendaylight.controller.md.sal.binding.api.ReadTransaction;
+import org.opendaylight.controller.md.sal.binding.api.ReadOnlyTransaction;
 import org.opendaylight.controller.md.sal.binding.api.ReadWriteTransaction;
 import org.opendaylight.controller.md.sal.binding.api.WriteTransaction;
 import org.opendaylight.controller.md.sal.common.api.data.AsyncTransaction;
@@ -129,8 +126,9 @@ final class TopologyNodeState implements AutoCloseable, TransactionChainListener
     }
 
     <T extends DataObject> ListenableFuture<Optional<T>> readOperationalData(final InstanceIdentifier<T> id) {
-        final ReadTransaction t = chain.newReadOnlyTransaction();
-        return t.read(LogicalDatastoreType.OPERATIONAL, id);
+        try (final ReadOnlyTransaction t = chain.newReadOnlyTransaction()) {
+            return t.read(LogicalDatastoreType.OPERATIONAL, id);
+        }
     }
 
     @Override

diff --git a/pcep/topology-provider/src/test/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractPCEPSessionTest.java b/pcep/topology-provider/src/test/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractPCEPSessionTest.java
index 956467b48c118447c7a5b0a6269c8df4b4804306..eba1ffe35e2856b2d2896fba61f7d704963c0a80 100644 (file)
--- a/pcep/topology-provider/src/test/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractPCEPSessionTest.java
+++ b/pcep/topology-provider/src/test/java/org/opendaylight/bgpcep/pcep/topology/provider/AbstractPCEPSessionTest.java
@@ -12,7 +12,6 @@ import static org.mockito.Matchers.any;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
-
 import com.google.common.base.Optional;
 import io.netty.channel.Channel;
 import io.netty.channel.ChannelFuture;
@@ -36,6 +35,7 @@ import org.mockito.Mockito;
 import org.mockito.MockitoAnnotations;
 import org.mockito.invocation.InvocationOnMock;
 import org.mockito.stubbing.Answer;
+import org.opendaylight.controller.md.sal.binding.api.ReadOnlyTransaction;
 import org.opendaylight.controller.md.sal.binding.test.AbstractDataBrokerTest;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.controller.md.sal.common.api.data.TransactionCommitFailedException;
@@ -140,7 +140,9 @@ public abstract class AbstractPCEPSessionTest<T extends TopologySessionListenerF
     }
 
     protected Optional<Topology> getTopology() throws InterruptedException, ExecutionException {
-        return getDataBroker().newReadOnlyTransaction().read(LogicalDatastoreType.OPERATIONAL, TOPO_IID).get();
+        try (ReadOnlyTransaction t = getDataBroker().newReadOnlyTransaction()) {
+            return t.read(LogicalDatastoreType.OPERATIONAL, TOPO_IID).get();
+        }
     }
 
     protected Ero createEroWithIpPrefixes(final List<String> ipPrefixes) {

diff --git a/pcep/tunnel-provider/src/main/java/org/opendaylight/bgpcep/pcep/tunnel/provider/TunnelProgramming.java b/pcep/tunnel-provider/src/main/java/org/opendaylight/bgpcep/pcep/tunnel/provider/TunnelProgramming.java
index 7bd10faf72cb8f5f5354a3c9da0eeeac4f12b1a3..5b9ba41878d51fb2a16588e1bb2eec474cd3926c 100644 (file)
--- a/pcep/tunnel-provider/src/main/java/org/opendaylight/bgpcep/pcep/tunnel/provider/TunnelProgramming.java
+++ b/pcep/tunnel-provider/src/main/java/org/opendaylight/bgpcep/pcep/tunnel/provider/TunnelProgramming.java
@@ -21,6 +21,7 @@ import org.opendaylight.bgpcep.programming.spi.SuccessfulRpcResult;
 import org.opendaylight.bgpcep.programming.topology.TopologyProgrammingUtil;
 import org.opendaylight.bgpcep.programming.tunnel.TunnelProgrammingUtil;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
+import org.opendaylight.controller.md.sal.binding.api.ReadOnlyTransaction;
 import org.opendaylight.controller.md.sal.binding.api.ReadTransaction;
 import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
 import org.opendaylight.controller.md.sal.common.api.data.ReadFailedException;
@@ -235,46 +236,45 @@ public final class TunnelProgramming implements TopologyTunnelPcepProgrammingSer
             protected ListenableFuture<OperationResult> invokeOperation() {
                 final InstanceIdentifier<Topology> tii = TopologyProgrammingUtil.topologyForInput(input);
 
-                final ReadTransaction t = TunnelProgramming.this.dataProvider.newReadOnlyTransaction();
+                try (final ReadOnlyTransaction t = TunnelProgramming.this.dataProvider.newReadOnlyTransaction()) {
+                    final TpReader dr = new TpReader(t, tii, input.getDestination());
+                    final TpReader sr = new TpReader(t, tii, input.getSource());
 
-                final TpReader dr = new TpReader(t, tii, input.getDestination());
-                final TpReader sr = new TpReader(t, tii, input.getSource());
+                    final Node sn = Preconditions.checkNotNull(sr.getNode());
+                    final TerminationPoint sp = Preconditions.checkNotNull(sr.getTp());
+                    final TerminationPoint dp = Preconditions.checkNotNull(dr.getTp());
 
-                final Node sn = Preconditions.checkNotNull(sr.getNode());
-                final TerminationPoint sp = Preconditions.checkNotNull(sr.getTp());
-                final TerminationPoint dp = Preconditions.checkNotNull(dr.getTp());
+                    final AddLspInputBuilder ab = new AddLspInputBuilder();
+                    ab.setNode(Preconditions.checkNotNull(supportingNode(sn)));
+                    ab.setName(input.getSymbolicPathName());
 
-                final AddLspInputBuilder ab = new AddLspInputBuilder();
-                ab.setNode(Preconditions.checkNotNull(supportingNode(sn)));
-                ab.setName(input.getSymbolicPathName());
-
-                // The link has to be non-existent
-                final InstanceIdentifier<Link> lii = NodeChangedListener.linkIdentifier(tii, ab.getNode(), ab.getName());
-                try {
-                    Preconditions.checkState(! t.read(LogicalDatastoreType.OPERATIONAL, lii).checkedGet().isPresent());
-                } catch (ReadFailedException e) {
-                    throw new IllegalStateException("Failed to ensure link existence.", e);
-                }
+                    // The link has to be non-existent
+                    final InstanceIdentifier<Link> lii = NodeChangedListener.linkIdentifier(tii, ab.getNode(), ab.getName());
+                    try {
+                        Preconditions.checkState(! t.read(LogicalDatastoreType.OPERATIONAL, lii).checkedGet().isPresent());
+                    } catch (ReadFailedException e) {
+                        throw new IllegalStateException("Failed to ensure link existence.", e);
+                    }
 
-                final ArgumentsBuilder args = new ArgumentsBuilder();
-                if (input.getBandwidth() != null) {
-                    args.setBandwidth(new BandwidthBuilder().setBandwidth(input.getBandwidth()).build());
-                }
-                if (input.getClassType() != null) {
-                    args.setClassType(new ClassTypeBuilder().setClassType(input.getClassType()).build());
-                }
-                args.setEndpointsObj(new EndpointsObjBuilder().setAddressFamily(buildAddressFamily(sp, dp)).build());
-                args.setEro(buildEro(input.getExplicitHops()));
-                args.setLspa(new LspaBuilder(input).build());
+                    final ArgumentsBuilder args = new ArgumentsBuilder();
+                    if (input.getBandwidth() != null) {
+                        args.setBandwidth(new BandwidthBuilder().setBandwidth(input.getBandwidth()).build());
+                    }
+                    if (input.getClassType() != null) {
+                        args.setClassType(new ClassTypeBuilder().setClassType(input.getClassType()).build());
+                    }
+                    args.setEndpointsObj(new EndpointsObjBuilder().setAddressFamily(buildAddressFamily(sp, dp)).build());
+                    args.setEro(buildEro(input.getExplicitHops()));
+                    args.setLspa(new LspaBuilder(input).build());
 
-                AdministrativeStatus adminStatus = input.getAugmentation(PcepCreateP2pTunnelInput1.class).getAdministrativeStatus();
-                if (adminStatus != null) {
-                    args.addAugmentation(Arguments2.class, new Arguments2Builder().setLsp(new LspBuilder().setAdministrative((adminStatus == AdministrativeStatus.Active) ? true : false).build()).build());
-                }
+                    AdministrativeStatus adminStatus = input.getAugmentation(PcepCreateP2pTunnelInput1.class).getAdministrativeStatus();
+                    if (adminStatus != null) {
+                        args.addAugmentation(Arguments2.class, new Arguments2Builder().setLsp(new LspBuilder().setAdministrative((adminStatus == AdministrativeStatus.Active) ? true : false).build()).build());
+                    }
 
-                ab.setArguments(args.build());
+                    ab.setArguments(args.build());
 
-                return Futures.transform(
+                    return Futures.transform(
                         (ListenableFuture<RpcResult<AddLspOutput>>) TunnelProgramming.this.topologyService.addLsp(ab.build()),
                         new Function<RpcResult<AddLspOutput>, OperationResult>() {
                             @Override
@@ -282,6 +282,7 @@ public final class TunnelProgramming implements TopologyTunnelPcepProgrammingSer
                                 return input.getResult();
                             }
                         });
+                }
             }
         }));
 
@@ -303,40 +304,40 @@ public final class TunnelProgramming implements TopologyTunnelPcepProgrammingSer
                 final InstanceIdentifier<Topology> tii = TopologyProgrammingUtil.topologyForInput(input);
                 final InstanceIdentifier<Link> lii = TunnelProgrammingUtil.linkIdentifier(tii, input);
 
-                final ReadTransaction t = TunnelProgramming.this.dataProvider.newReadOnlyTransaction();
-                final Node node;
-                final Link link;
-
-                try {
-                    // The link has to exist
-                    link = (Link) t.read(LogicalDatastoreType.OPERATIONAL, lii).checkedGet().get();
-
-                    // The source node has to exist
-                    node = sourceNode(t, tii, link).get();
-                } catch (IllegalStateException | ReadFailedException e) {
-                    return Futures.<OperationResult>immediateFuture(new OperationResult() {
-                        @Override
-                        public Class<? extends DataContainer> getImplementedInterface() {
-                            return OperationResult.class;
-                        }
-
-                        @Override
-                        public FailureType getFailure() {
-                            return FailureType.Unsent;
-                        }
-
-                        @Override
-                        public List<Error> getError() {
-                            return Collections.emptyList();
-                        }
-                    });
-                }
+                try (final ReadOnlyTransaction t = TunnelProgramming.this.dataProvider.newReadOnlyTransaction()) {
+                    final Node node;
+                    final Link link;
+
+                    try {
+                        // The link has to exist
+                        link = t.read(LogicalDatastoreType.OPERATIONAL, lii).checkedGet().get();
+
+                        // The source node has to exist
+                        node = sourceNode(t, tii, link).get();
+                    } catch (IllegalStateException | ReadFailedException e) {
+                        return Futures.<OperationResult>immediateFuture(new OperationResult() {
+                            @Override
+                            public Class<? extends DataContainer> getImplementedInterface() {
+                                return OperationResult.class;
+                            }
+
+                            @Override
+                            public FailureType getFailure() {
+                                return FailureType.Unsent;
+                            }
+
+                            @Override
+                            public List<Error> getError() {
+                                return Collections.emptyList();
+                            }
+                        });
+                    }
 
-                final RemoveLspInputBuilder ab = new RemoveLspInputBuilder();
-                ab.setName(link.getAugmentation(Link1.class).getSymbolicPathName());
-                ab.setNode(node.getSupportingNode().get(0).getKey().getNodeRef());
+                    final RemoveLspInputBuilder ab = new RemoveLspInputBuilder();
+                    ab.setName(link.getAugmentation(Link1.class).getSymbolicPathName());
+                    ab.setNode(node.getSupportingNode().get(0).getKey().getNodeRef());
 
-                return Futures.transform(
+                    return Futures.transform(
                         (ListenableFuture<RpcResult<RemoveLspOutput>>) TunnelProgramming.this.topologyService.removeLsp(ab.build()),
                         new Function<RpcResult<RemoveLspOutput>, OperationResult>() {
                             @Override
@@ -344,6 +345,7 @@ public final class TunnelProgramming implements TopologyTunnelPcepProgrammingSer
                                 return input.getResult();
                             }
                         });
+                }
             }
         }));
 
@@ -361,53 +363,53 @@ public final class TunnelProgramming implements TopologyTunnelPcepProgrammingSer
                 final InstanceIdentifier<Topology> tii = TopologyProgrammingUtil.topologyForInput(input);
                 final InstanceIdentifier<Link> lii = TunnelProgrammingUtil.linkIdentifier(tii, input);
 
-                final ReadTransaction t = TunnelProgramming.this.dataProvider.newReadOnlyTransaction();
-                final Link link;
-                final Node node;
-
-                try {
-                    // The link has to exist
-                    link = (Link) t.read(LogicalDatastoreType.OPERATIONAL, lii).checkedGet().get();
-
-                    // The source node has to exist
-                    node = sourceNode(t, tii, link).get();
-                } catch (IllegalStateException | ReadFailedException e) {
-                    return Futures.<OperationResult>immediateFuture(new OperationResult() {
-                        @Override
-                        public Class<? extends DataContainer> getImplementedInterface() {
-                            return OperationResult.class;
-                        }
-
-                        @Override
-                        public FailureType getFailure() {
-                            return FailureType.Unsent;
-                        }
-
-                        @Override
-                        public List<Error> getError() {
-                            return Collections.emptyList();
-                        }
-                    });
-                }
+                try (final ReadOnlyTransaction t = TunnelProgramming.this.dataProvider.newReadOnlyTransaction()) {
+                    final Link link;
+                    final Node node;
 
-                final UpdateLspInputBuilder ab = new UpdateLspInputBuilder();
-                ab.setName(link.getAugmentation(Link1.class).getSymbolicPathName());
-                ab.setNode(Preconditions.checkNotNull(supportingNode(node)));
+                    try {
+                        // The link has to exist
+                        link = t.read(LogicalDatastoreType.OPERATIONAL, lii).checkedGet().get();
 
-                final org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.topology.pcep.rev131024.update.lsp.args.ArgumentsBuilder args = new org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.topology.pcep.rev131024.update.lsp.args.ArgumentsBuilder();
-                args.setBandwidth(new BandwidthBuilder().setBandwidth(input.getBandwidth()).build());
-                args.setClassType(new ClassTypeBuilder().setClassType(input.getClassType()).build());
-                args.setEro(buildEro(input.getExplicitHops()));
-                args.setLspa(new LspaBuilder(input).build());
+                        // The source node has to exist
+                        node = sourceNode(t, tii, link).get();
+                    } catch (IllegalStateException | ReadFailedException e) {
+                        return Futures.<OperationResult>immediateFuture(new OperationResult() {
+                            @Override
+                            public Class<? extends DataContainer> getImplementedInterface() {
+                                return OperationResult.class;
+                            }
 
-                AdministrativeStatus adminStatus = input.getAugmentation(PcepUpdateTunnelInput1.class).getAdministrativeStatus();
-                if (adminStatus != null) {
-                    args.addAugmentation(Arguments3.class, new Arguments3Builder().setLsp(new LspBuilder().setAdministrative((adminStatus == AdministrativeStatus.Active) ? true : false).build()).build());
-                }
+                            @Override
+                            public FailureType getFailure() {
+                                return FailureType.Unsent;
+                            }
+
+                            @Override
+                            public List<Error> getError() {
+                                return Collections.emptyList();
+                            }
+                        });
+                    }
 
-                ab.setArguments(args.build());
+                    final UpdateLspInputBuilder ab = new UpdateLspInputBuilder();
+                    ab.setName(link.getAugmentation(Link1.class).getSymbolicPathName());
+                    ab.setNode(Preconditions.checkNotNull(supportingNode(node)));
 
-                return Futures.transform(
+                    final org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.topology.pcep.rev131024.update.lsp.args.ArgumentsBuilder args = new org.opendaylight.yang.gen.v1.urn.opendaylight.params.xml.ns.yang.topology.pcep.rev131024.update.lsp.args.ArgumentsBuilder();
+                    args.setBandwidth(new BandwidthBuilder().setBandwidth(input.getBandwidth()).build());
+                    args.setClassType(new ClassTypeBuilder().setClassType(input.getClassType()).build());
+                    args.setEro(buildEro(input.getExplicitHops()));
+                    args.setLspa(new LspaBuilder(input).build());
+
+                    AdministrativeStatus adminStatus = input.getAugmentation(PcepUpdateTunnelInput1.class).getAdministrativeStatus();
+                    if (adminStatus != null) {
+                        args.addAugmentation(Arguments3.class, new Arguments3Builder().setLsp(new LspBuilder().setAdministrative((adminStatus == AdministrativeStatus.Active) ? true : false).build()).build());
+                    }
+
+                    ab.setArguments(args.build());
+
+                    return Futures.transform(
                         (ListenableFuture<RpcResult<UpdateLspOutput>>) TunnelProgramming.this.topologyService.updateLsp(ab.build()),
                         new Function<RpcResult<UpdateLspOutput>, OperationResult>() {
                             @Override
@@ -415,6 +417,7 @@ public final class TunnelProgramming implements TopologyTunnelPcepProgrammingSer
                                 return input.getResult();
                             }
                         });
+                }
             }
         }));