From c813b399f31c0f7424be126fbf723df6a111cb43 Mon Sep 17 00:00:00 2001 From: tpantelis Date: Tue, 22 Apr 2014 12:05:23 -0400 Subject: [PATCH] Bug 716: Errors on controller shutdown Fixed miscellaneous errors on shutdown: - FlowProvider, GroupProvider and MeterProvider: all throw an UnsupportedOperationEx from their close methods with a TODO message. Implemented close to close the commitHandlerRegistration. - NetconfSSHServer: logs a socket closed exception due to closing the socket on shutdown. We can ignore this and not log the error (if 'up' is false). - RestconfProvider: NPE in stop method calling session.close. The 'session' member is never initialized or otherwise used so removed it. - RuntimeMappingModule and SchemaServiceImplSingletonModule: IllegalStateException from bundleContext.ungetService() call because the bundle has already been shutdown. Ignore ex as this can occur normally. Need someone to commit this! Change-Id: I31d9d6d66418dda7f5b73c2ad12bb251f3689643 Signed-off-by: tpantelis --- .../binding/impl/RuntimeMappingModule.java | 15 +++++++++++++-- .../SchemaServiceImplSingletonModule.java | 19 ++++++++++++++++++- .../sal/rest/impl/RestconfProvider.java | 2 -- .../netconf/ssh/NetconfSSHServer.java | 16 ++++++++++++---- 4 files changed, 43 insertions(+), 9 deletions(-) diff --git a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/config/yang/md/sal/binding/impl/RuntimeMappingModule.java b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/config/yang/md/sal/binding/impl/RuntimeMappingModule.java index 750defc0e9..0f0ce0dc9d 100644 --- a/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/config/yang/md/sal/binding/impl/RuntimeMappingModule.java +++ b/opendaylight/md-sal/sal-binding-broker/src/main/java/org/opendaylight/controller/config/yang/md/sal/binding/impl/RuntimeMappingModule.java @@ -26,6 +26,8 @@ import org.opendaylight.yangtools.yang.data.impl.codec.DeserializationException; import org.opendaylight.yangtools.yang.model.api.SchemaServiceListener; import org.osgi.framework.BundleContext; import org.osgi.framework.ServiceReference; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import com.google.common.base.Optional; import com.google.common.base.Preconditions; @@ -36,6 +38,8 @@ import com.google.common.base.Preconditions; public final class RuntimeMappingModule extends org.opendaylight.controller.config.yang.md.sal.binding.impl.AbstractRuntimeMappingModule { + private static final Logger LOG = LoggerFactory.getLogger(RuntimeMappingModule.class); + private BundleContext bundleContext; public RuntimeMappingModule(org.opendaylight.controller.config.api.ModuleIdentifier identifier, @@ -163,10 +167,17 @@ public final class RuntimeMappingModule extends } @Override - public void close() throws Exception { + public void close() { if(delegate != null) { delegate = null; - bundleContext.ungetService(reference); + + try { + bundleContext.ungetService(reference); + } catch (IllegalStateException e) { + // Indicates the BundleContext is no longer valid which can happen normally on shutdown. + LOG.debug( "Error unregistering service", e ); + } + bundleContext= null; reference = null; } diff --git a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/config/yang/md/sal/dom/impl/SchemaServiceImplSingletonModule.java b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/config/yang/md/sal/dom/impl/SchemaServiceImplSingletonModule.java index ba7414d42e..93284c98e1 100644 --- a/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/config/yang/md/sal/dom/impl/SchemaServiceImplSingletonModule.java +++ b/opendaylight/md-sal/sal-dom-broker/src/main/java/org/opendaylight/controller/config/yang/md/sal/dom/impl/SchemaServiceImplSingletonModule.java @@ -16,6 +16,8 @@ import org.opendaylight.yangtools.yang.model.api.SchemaContext; import org.opendaylight.yangtools.yang.model.api.SchemaServiceListener; import org.osgi.framework.BundleContext; import org.osgi.framework.ServiceReference; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** * @@ -23,6 +25,8 @@ import org.osgi.framework.ServiceReference; public final class SchemaServiceImplSingletonModule extends org.opendaylight.controller.config.yang.md.sal.dom.impl.AbstractSchemaServiceImplSingletonModule { + private static final Logger LOG = LoggerFactory.getLogger(SchemaServiceImplSingletonModule.class); + BundleContext bundleContext; public SchemaServiceImplSingletonModule(org.opendaylight.controller.config.api.ModuleIdentifier identifier, @@ -83,28 +87,41 @@ public final class SchemaServiceImplSingletonModule extends public void close() throws Exception { if (delegate != null) { delegate = null; - bundleContext.ungetService(reference); + + try { + bundleContext.ungetService(reference); + } catch (IllegalStateException e) { + // Indicates the service was already unregistered which can happen normally + // on shutdown. + LOG.debug( "Error unregistering service", e ); + } + reference = null; bundleContext = null; } } + @Override public void addModule(Module arg0) { delegate.addModule(arg0); } + @Override public SchemaContext getGlobalContext() { return delegate.getGlobalContext(); } + @Override public SchemaContext getSessionContext() { return delegate.getSessionContext(); } + @Override public ListenerRegistration registerSchemaServiceListener(SchemaServiceListener arg0) { return delegate.registerSchemaServiceListener(arg0); } + @Override public void removeModule(Module arg0) { delegate.removeModule(arg0); } diff --git a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/RestconfProvider.java b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/RestconfProvider.java index 1870bdf0bf..2abd4b6a3a 100644 --- a/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/RestconfProvider.java +++ b/opendaylight/md-sal/sal-rest-connector/src/main/java/org/opendaylight/controller/sal/rest/impl/RestconfProvider.java @@ -34,7 +34,6 @@ public class RestconfProvider implements BundleActivator, Provider, ServiceTrack private ListenerRegistration listenerRegistration; private ServiceTracker brokerServiceTrancker; private BundleContext bundleContext; - private ProviderSession session; private Thread webSocketServerThread; @Override @@ -70,7 +69,6 @@ public class RestconfProvider implements BundleActivator, Provider, ServiceTrack } } webSocketServerThread.interrupt(); - session.close(); brokerServiceTrancker.close(); } diff --git a/opendaylight/netconf/netconf-ssh/src/main/java/org/opendaylight/controller/netconf/ssh/NetconfSSHServer.java b/opendaylight/netconf/netconf-ssh/src/main/java/org/opendaylight/controller/netconf/ssh/NetconfSSHServer.java index ff950e95e9..c6974d4982 100644 --- a/opendaylight/netconf/netconf-ssh/src/main/java/org/opendaylight/controller/netconf/ssh/NetconfSSHServer.java +++ b/opendaylight/netconf/netconf-ssh/src/main/java/org/opendaylight/controller/netconf/ssh/NetconfSSHServer.java @@ -27,7 +27,7 @@ public final class NetconfSSHServer implements Runnable { private static final AtomicLong sesssionId = new AtomicLong(); private final InetSocketAddress clientAddress; private final AuthProvider authProvider; - private boolean up = false; + private volatile boolean up = false; private NetconfSSHServer(int serverPort,InetSocketAddress clientAddress, AuthProvider authProvider) throws IllegalStateException, IOException { @@ -68,9 +68,17 @@ public final class NetconfSSHServer implements Runnable { while (up) { logger.trace("Starting new socket thread."); try { - SocketThread.start(ss.accept(), clientAddress, sesssionId.incrementAndGet(), authProvider); - } catch (IOException e) { - logger.error("Exception occurred during socket thread initialization {}", e); + SocketThread.start(ss.accept(), clientAddress, sesssionId.incrementAndGet(), authProvider); + } + catch (IOException e) { + if( up ) { + logger.error("Exception occurred during socket thread initialization", e); + } + else { + // We're shutting down so an exception is expected as the socket's been closed. + // Log to debug. + logger.debug("Shutting down - got expected exception: " + e); + } } } } -- 2.36.6