BUG-732 Fix netconf client deadlock when downloading yang schemas from remote device

Signed-off-by: Maros Marsalek <mmarsale@cisco.com>
Change-Id: Ic70085655e38ebc1e159fd4ba4b93172e468f4ad

diff --git a/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/NetconfDevice.java b/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/NetconfDevice.java
index abbbb68265e3209915f32f17e589ff76c4dc2a91..4ea0fa5645b6fe2a772b82a9b0dc72757a7c4edd 100644 (file)
--- a/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/NetconfDevice.java
+++ b/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/NetconfDevice.java
@@ -172,30 +172,40 @@ public class NetconfDevice implements Provider, //
         }
     }
 
-    void bringUp(SchemaSourceProvider<String> delegate, Set<QName> capabilities) {
-        remoteSourceProvider = schemaSourceProvider.createInstanceFor(delegate);
-        deviceContextProvider = new NetconfDeviceSchemaContextProvider(this, remoteSourceProvider);
-        deviceContextProvider.createContextFromCapabilities(capabilities);
-        if (mountInstance != null && getSchemaContext().isPresent()) {
-            mountInstance.setSchemaContext(getSchemaContext().get());
-        }
+    void bringUp(final SchemaSourceProvider<String> delegate, final Set<QName> capabilities) {
+        // This has to be called from separate thread, not from netty thread calling onSessionUp in DeviceListener.
+        // Reason: delegate.getSchema blocks thread when waiting for response
+        // however, if the netty thread is blocked, no incoming message can be processed
+        // ... netty should pick another thread from pool to process incoming message, but it does not http://netty.io/wiki/thread-model.html
+        // TODO redesign +refactor
+        processingExecutor.submit(new Runnable() {
+            @Override
+            public void run() {
+                remoteSourceProvider = schemaSourceProvider.createInstanceFor(delegate);
+                deviceContextProvider = new NetconfDeviceSchemaContextProvider(NetconfDevice.this, remoteSourceProvider);
+                deviceContextProvider.createContextFromCapabilities(capabilities);
+                if (mountInstance != null && getSchemaContext().isPresent()) {
+                    mountInstance.setSchemaContext(getSchemaContext().get());
+                }
 
-        updateDeviceState(true, capabilities);
+                updateDeviceState(true, capabilities);
 
-        if (mountInstance != null) {
-            confReaderReg = mountInstance.registerConfigurationReader(ROOT_PATH, this);
-            operReaderReg = mountInstance.registerOperationalReader(ROOT_PATH, this);
-            commitHandlerReg = mountInstance.registerCommitHandler(ROOT_PATH, this);
+                if (mountInstance != null) {
+                    confReaderReg = mountInstance.registerConfigurationReader(ROOT_PATH, NetconfDevice.this);
+                    operReaderReg = mountInstance.registerOperationalReader(ROOT_PATH, NetconfDevice.this);
+                    commitHandlerReg = mountInstance.registerCommitHandler(ROOT_PATH, NetconfDevice.this);
 
-            List<RpcRegistration> rpcs = new ArrayList<>();
-            // TODO same condition twice
-            if (mountInstance != null && getSchemaContext().isPresent()) {
-                for (RpcDefinition rpc : mountInstance.getSchemaContext().getOperations()) {
-                    rpcs.add(mountInstance.addRpcImplementation(rpc.getQName(), this));
+                    List<RpcRegistration> rpcs = new ArrayList<>();
+                    // TODO same condition twice
+                    if (mountInstance != null && getSchemaContext().isPresent()) {
+                        for (RpcDefinition rpc : mountInstance.getSchemaContext().getOperations()) {
+                            rpcs.add(mountInstance.addRpcImplementation(rpc.getQName(), NetconfDevice.this));
+                        }
+                    }
+                    rpcReg = rpcs;
                 }
             }
-            rpcReg = rpcs;
-        }
+        });
     }
 
     private void updateDeviceState(boolean up, Set<QName> capabilities) {

diff --git a/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/NetconfMapping.java b/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/NetconfMapping.java
index ce2661a7b3f9cfcacc2dd6bf9f930fd1bd0b0597..a8ef4dd2fb262a7ae1e4383faa64d2d24b4e8673 100644 (file)
--- a/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/NetconfMapping.java
+++ b/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/NetconfMapping.java
@@ -143,8 +143,7 @@ public class NetconfMapping {
         try {
             w3cPayload = XmlDocumentUtils.toDocument(rpcPayload, XmlDocumentUtils.defaultValueCodecProvider());
         } catch (UnsupportedDataTypeException e) {
-            // FIXME Ex handling
-            e.printStackTrace();
+            throw new IllegalArgumentException("Unable to create message", e);
         }
         w3cPayload.getDocumentElement().setAttribute("message-id", "m-" + messageId.getAndIncrement());
         return new NetconfMessage(w3cPayload);
@@ -249,7 +248,7 @@ public class NetconfMapping {
     public static void checkValidReply(NetconfMessage input, NetconfMessage output) {
         String inputMsgId = input.getDocument().getDocumentElement().getAttribute("message-id");
         String outputMsgId = output.getDocument().getDocumentElement().getAttribute("message-id");
-        Preconditions.checkState(inputMsgId == outputMsgId, "Rpc request and reply message IDs must be same.");
+        Preconditions.checkState(inputMsgId.equals(outputMsgId), "Rpc request and reply message IDs must be same.");
     }
 
 }