From fb6dc734bda582b4d77360dbc8a97a5ecd482d39 Mon Sep 17 00:00:00 2001 From: PeterSuna Date: Wed, 7 Oct 2020 10:52:14 +0200 Subject: [PATCH] Add SchemaSourceRegistration list to NetconfConnectorDTO When a NetconfDevice is initialized, we keep track of it via NetconfConnectorDTO. As a device actually can have some YANG Library source attached to it, we are registering them as well, but we are failing to unregister them. Keep the registration list in the DTO and route close requests towards it -- allowing us to also remove the registrations. JIRA: NETCONF-675 Change-Id: Id1d5fb2ff23aad766273e22d5f6f6639300fb3a5 Signed-off-by: PeterSuna Signed-off-by: Robert Varga --- .../topology/spi/AbstractNetconfTopology.java | 85 +++++++++---------- 1 file changed, 38 insertions(+), 47 deletions(-) diff --git a/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/AbstractNetconfTopology.java b/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/AbstractNetconfTopology.java index cb5249fef4..d52010e4ee 100644 --- a/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/AbstractNetconfTopology.java +++ b/netconf/netconf-topology/src/main/java/org/opendaylight/netconf/topology/spi/AbstractNetconfTopology.java @@ -11,7 +11,6 @@ import static java.util.Objects.requireNonNull; import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Preconditions; -import com.google.common.collect.Lists; import com.google.common.util.concurrent.FutureCallback; import com.google.common.util.concurrent.Futures; import com.google.common.util.concurrent.ListenableFuture; @@ -46,7 +45,6 @@ import org.opendaylight.netconf.sal.connect.api.RemoteDevice; import org.opendaylight.netconf.sal.connect.api.RemoteDeviceHandler; import org.opendaylight.netconf.sal.connect.api.SchemaResourceManager; import org.opendaylight.netconf.sal.connect.netconf.LibraryModulesSchemas; -import org.opendaylight.netconf.sal.connect.netconf.NetconfDevice; import org.opendaylight.netconf.sal.connect.netconf.NetconfDevice.SchemaResourcesDTO; import org.opendaylight.netconf.sal.connect.netconf.NetconfDeviceBuilder; import org.opendaylight.netconf.sal.connect.netconf.SchemalessNetconfDevice; @@ -101,7 +99,6 @@ public abstract class AbstractNetconfTopology implements NetconfTopology { private static final long DEFAULT_CONNECTION_TIMEOUT_MILLIS = 20000L; private static final BigDecimal DEFAULT_SLEEP_FACTOR = new BigDecimal(1.5); - private final NetconfClientDispatcher clientDispatcher; private final EventExecutor eventExecutor; private final DeviceActionFactory deviceActionFactory; @@ -165,15 +162,14 @@ public abstract class AbstractNetconfTopology implements NetconfTopology { @Override public ListenableFuture disconnectNode(final NodeId nodeId) { LOG.debug("Disconnecting RemoteDevice{{}}", nodeId.getValue()); - if (!activeConnectors.containsKey(nodeId)) { + + final NetconfConnectorDTO connectorDTO = activeConnectors.remove(nodeId); + if (connectorDTO == null) { return Futures.immediateFailedFuture( - new IllegalStateException("Unable to disconnect device that is not connected")); + new IllegalStateException("Unable to disconnect device that is not connected")); } - // retrieve connection, and disconnect it - final NetconfConnectorDTO connectorDTO = activeConnectors.remove(nodeId); - connectorDTO.getCommunicator().close(); - connectorDTO.getFacade().close(); + connectorDTO.close(); return Futures.immediateFuture(null); } @@ -238,10 +234,28 @@ public abstract class AbstractNetconfTopology implements NetconfTopology { } final RemoteDevice device; + final List> yanglibRegistrations; if (node.isSchemaless()) { device = new SchemalessNetconfDevice(baseSchemas, remoteDeviceId, salFacade); + yanglibRegistrations = List.of(); } else { - device = createNetconfDevice(remoteDeviceId, salFacade, nodeId, node, nodeOptional); + final boolean reconnectOnChangedSchema = node.isReconnectOnChangedSchema() == null + ? DEFAULT_RECONNECT_ON_CHANGED_SCHEMA : node.isReconnectOnChangedSchema(); + + final SchemaResourcesDTO resources = schemaManager.getSchemaResources(node, nodeId.getValue()); + device = new NetconfDeviceBuilder() + .setReconnectOnSchemasChange(reconnectOnChangedSchema) + .setSchemaResourcesDTO(resources) + .setGlobalProcessingExecutor(this.processingExecutor) + .setId(remoteDeviceId) + .setSalFacade(salFacade) + .setNode(node) + .setEventExecutor(eventExecutor) + .setNodeOptional(nodeOptional) + .setDeviceActionFactory(deviceActionFactory) + .setBaseSchemas(baseSchemas) + .build(); + yanglibRegistrations = registerDeviceSchemaSources(remoteDeviceId, nodeId, node, resources); } final Optional userCapabilities = getUserCapabilities(node); @@ -260,44 +274,17 @@ public abstract class AbstractNetconfTopology implements NetconfTopology { if (salFacade instanceof KeepaliveSalFacade) { ((KeepaliveSalFacade)salFacade).setListener(netconfDeviceCommunicator); } - return new NetconfConnectorDTO(netconfDeviceCommunicator, salFacade); - } - private NetconfDevice createNetconfDevice(final RemoteDeviceId remoteDeviceId, - final RemoteDeviceHandler salFacade, final NodeId nodeId, final NetconfNode node, - final NetconfNodeAugmentedOptional nodeOptional) { - final boolean reconnectOnChangedSchema = node.isReconnectOnChangedSchema() == null - ? DEFAULT_RECONNECT_ON_CHANGED_SCHEMA : node.isReconnectOnChangedSchema(); - - final SchemaResourcesDTO resources = schemaManager.getSchemaResources(node, nodeId.getValue()); - - final NetconfDevice device = new NetconfDeviceBuilder() - .setReconnectOnSchemasChange(reconnectOnChangedSchema) - .setSchemaResourcesDTO(resources) - .setGlobalProcessingExecutor(this.processingExecutor) - .setId(remoteDeviceId) - .setSalFacade(salFacade) - .setNode(node) - .setEventExecutor(eventExecutor) - .setNodeOptional(nodeOptional) - .setDeviceActionFactory(deviceActionFactory) - .setBaseSchemas(baseSchemas) - .build(); + return new NetconfConnectorDTO(netconfDeviceCommunicator, salFacade, yanglibRegistrations); + } + private List> registerDeviceSchemaSources(final RemoteDeviceId remoteDeviceId, + final NodeId nodeId, final NetconfNode node, final SchemaResourcesDTO resources) { final YangLibrary yangLibrary = node.getYangLibrary(); if (yangLibrary != null) { final Uri uri = yangLibrary.getYangLibraryUrl(); if (uri != null) { - // FIXME: NETCONF-675: these registrations need to be torn down with the device. This does not look - // quite right, though, as we can end up adding a lot of registrations on a - // per-device basis. - // This leak is also detected by SpotBugs as soon as this initialization is switched - // to proper "new ArrayList<>" and hence we really need to attach these somewhere - // else. - // It seems we should be subclassing NetconfConnectorDTO for this purpose as a - // first step and then perhaps do some refcounting or similar based on the - // schemaRegistry instance. - final List> registeredYangLibSources = Lists.newArrayList(); + final List> registrations = new ArrayList<>(); final String yangLibURL = uri.getValue(); final SchemaSourceRegistry schemaRegistry = resources.getSchemaRegistry(); @@ -312,15 +299,16 @@ public abstract class AbstractNetconfTopology implements NetconfTopology { } for (final Map.Entry entry : schemas.getAvailableModels().entrySet()) { - registeredYangLibSources.add(schemaRegistry.registerSchemaSource( - new YangLibrarySchemaYangSourceProvider(remoteDeviceId, schemas.getAvailableModels()), + registrations.add(schemaRegistry.registerSchemaSource(new YangLibrarySchemaYangSourceProvider( + remoteDeviceId, schemas.getAvailableModels()), PotentialSchemaSource.create(entry.getKey(), YangTextSchemaSource.class, PotentialSchemaSource.Costs.REMOTE_IO.getValue()))); } + return registrations; } } - return device; + return List.of(); } /** @@ -459,14 +447,16 @@ public abstract class AbstractNetconfTopology implements NetconfTopology { } protected static class NetconfConnectorDTO implements AutoCloseable { - + private final List> yanglibRegistrations; private final NetconfDeviceCommunicator communicator; private final RemoteDeviceHandler facade; public NetconfConnectorDTO(final NetconfDeviceCommunicator communicator, - final RemoteDeviceHandler facade) { + final RemoteDeviceHandler facade, + final List> yanglibRegistrations) { this.communicator = communicator; this.facade = facade; + this.yanglibRegistrations = yanglibRegistrations; } public NetconfDeviceCommunicator getCommunicator() { @@ -485,6 +475,7 @@ public abstract class AbstractNetconfTopology implements NetconfTopology { public void close() { communicator.close(); facade.close(); + yanglibRegistrations.forEach(SchemaSourceRegistration::close); } } } -- 2.36.6