From 46392456c5318c713e74e684b54119913f6fedb5 Mon Sep 17 00:00:00 2001 From: Maros Marsalek Date: Mon, 14 Jul 2014 18:04:53 +0200 Subject: [PATCH] BUG-1317 BUG-1351 Fix duplicate connections from netconf connector Change-Id: I2c3135f3f84ef25f0005744c03003462c96812c1 Signed-off-by: Maros Marsalek --- .../listener/NetconfDeviceCommunicator.java | 4 +-- .../netconf/sal/NetconfDeviceSalProvider.java | 8 ++--- .../AbstractNetconfSessionNegotiator.java | 30 ++++++++++++++----- 3 files changed, 27 insertions(+), 15 deletions(-) diff --git a/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/listener/NetconfDeviceCommunicator.java b/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/listener/NetconfDeviceCommunicator.java index 9fa68eeea6..f36ad9abcb 100644 --- a/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/listener/NetconfDeviceCommunicator.java +++ b/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/listener/NetconfDeviceCommunicator.java @@ -86,9 +86,9 @@ public class NetconfDeviceCommunicator implements NetconfClientSessionListener, final NetconfClientConfiguration config) { if(config instanceof NetconfReconnectingClientConfiguration) { dispatch.createReconnectingClient((NetconfReconnectingClientConfiguration) config); + } else { + dispatch.createClient(config); } - - dispatch.createClient(config); } private void tearDown( String reason ) { diff --git a/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/sal/NetconfDeviceSalProvider.java b/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/sal/NetconfDeviceSalProvider.java index 01af84c9ac..fc54bfbc3d 100644 --- a/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/sal/NetconfDeviceSalProvider.java +++ b/opendaylight/md-sal/sal-netconf-connector/src/main/java/org/opendaylight/controller/sal/connect/netconf/sal/NetconfDeviceSalProvider.java @@ -7,9 +7,9 @@ */ package org.opendaylight.controller.sal.connect.netconf.sal; +import com.google.common.base.Preconditions; import java.util.Collection; import java.util.Collections; - import java.util.concurrent.ExecutorService; import org.opendaylight.controller.sal.binding.api.BindingAwareBroker; import org.opendaylight.controller.sal.binding.api.BindingAwareProvider; @@ -23,8 +23,6 @@ import org.opendaylight.yangtools.yang.binding.RpcService; import org.slf4j.Logger; import org.slf4j.LoggerFactory; -import com.google.common.base.Preconditions; - final class NetconfDeviceSalProvider implements AutoCloseable, Provider, BindingAwareProvider { private static final Logger logger = LoggerFactory.getLogger(NetconfDeviceSalProvider.class); @@ -41,13 +39,13 @@ final class NetconfDeviceSalProvider implements AutoCloseable, Provider, Binding public MountProvisionInstance getMountInstance() { Preconditions.checkState(mountInstance != null, - "%s: Sal provider was not initialized by sal. Cannot publish notification", id); + "%s: Sal provider was not initialized by sal. Cannot get mount instance", id); return mountInstance; } public NetconfDeviceDatastoreAdapter getDatastoreAdapter() { Preconditions.checkState(datastoreAdapter != null, - "%s: Sal provider %s was not initialized by sal. Cannot publish notification", id); + "%s: Sal provider %s was not initialized by sal. Cannot get datastore adapter", id); return datastoreAdapter; } diff --git a/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/AbstractNetconfSessionNegotiator.java b/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/AbstractNetconfSessionNegotiator.java index c770bde920..1360a54d6f 100644 --- a/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/AbstractNetconfSessionNegotiator.java +++ b/opendaylight/netconf/netconf-netty-util/src/main/java/org/opendaylight/controller/netconf/nettyutil/AbstractNetconfSessionNegotiator.java @@ -60,6 +60,7 @@ extends AbstractSessionNegotiator { } private State state = State.IDLE; + private final Promise promise; private final Timer timer; private final long connectionTimeoutMillis; @@ -68,6 +69,7 @@ extends AbstractSessionNegotiator { L sessionListener, long connectionTimeoutMillis) { super(promise, channel); this.sessionPreferences = sessionPreferences; + this.promise = promise; this.timer = timer; this.sessionListener = sessionListener; this.connectionTimeoutMillis = connectionTimeoutMillis; @@ -106,28 +108,40 @@ extends AbstractSessionNegotiator { channel.pipeline().addLast(NAME_OF_EXCEPTION_HANDLER, new ExceptionHandlingInboundChannelHandler()); + // FIXME, make sessionPreferences return HelloMessage, move NetconfHelloMessage to API + sendMessage((NetconfHelloMessage)helloMessage); + + replaceHelloMessageOutboundHandler(); + changeState(State.OPEN_WAIT); + timeout = this.timer.newTimeout(new TimerTask() { @Override public void run(final Timeout timeout) { synchronized (this) { if (state != State.ESTABLISHED) { + logger.debug("Connection timeout after {}, session is in state {}", timeout, state); - final IllegalStateException cause = new IllegalStateException( - "Session was not established after " + timeout); - negotiationFailed(cause); + + // Do not fail negotiation if promise is done or canceled + // It would result in setting result of the promise second time and that throws exception + if (isPromiseFinished() == false) { + // FIXME BUG-1365 calling "negotiation failed" closes the channel, but the channel does not get closed if data is still being transferred + // Loopback connection initiation might + negotiationFailed(new IllegalStateException("Session was not established after " + timeout)); + } + changeState(State.FAILED); } else if(channel.isOpen()) { channel.pipeline().remove(NAME_OF_EXCEPTION_HANDLER); } } } - }, connectionTimeoutMillis, TimeUnit.MILLISECONDS); - // FIXME, make sessionPreferences return HelloMessage, move NetconfHelloMessage to API - sendMessage((NetconfHelloMessage)helloMessage); + private boolean isPromiseFinished() { + return promise.isDone() || promise.isCancelled(); + } - replaceHelloMessageOutboundHandler(); - changeState(State.OPEN_WAIT); + }, connectionTimeoutMillis, TimeUnit.MILLISECONDS); } private void cancelTimeout() { -- 2.36.6