import io.netty.util.concurrent.Future;
import io.netty.util.concurrent.FutureListener;
import java.io.IOException;
+import java.lang.invoke.MethodHandles;
+import java.lang.invoke.VarHandle;
import java.net.SocketAddress;
+import java.time.Duration;
import java.util.concurrent.TimeUnit;
-import java.util.concurrent.atomic.AtomicBoolean;
+import org.checkerframework.checker.lock.qual.GuardedBy;
+import org.checkerframework.checker.lock.qual.Holding;
import org.eclipse.jdt.annotation.Nullable;
import org.opendaylight.netconf.nettyutil.handler.ssh.authentication.AuthenticationHandler;
+import org.opendaylight.netconf.shaded.sshd.client.channel.ChannelSubsystem;
import org.opendaylight.netconf.shaded.sshd.client.channel.ClientChannel;
import org.opendaylight.netconf.shaded.sshd.client.future.AuthFuture;
import org.opendaylight.netconf.shaded.sshd.client.future.ConnectFuture;
+import org.opendaylight.netconf.shaded.sshd.client.future.OpenFuture;
import org.opendaylight.netconf.shaded.sshd.client.session.ClientSession;
import org.opendaylight.netconf.shaded.sshd.core.CoreModuleProperties;
import org.slf4j.Logger;
/**
* Netty SSH handler class. Acts as interface between Netty and SSH library.
*/
-public class AsyncSshHandler extends ChannelOutboundHandlerAdapter {
+@Deprecated(since = "7.0.0", forRemoval = true)
+public final class AsyncSshHandler extends ChannelOutboundHandlerAdapter {
private static final Logger LOG = LoggerFactory.getLogger(AsyncSshHandler.class);
+ private static final VarHandle DISCONNECTED;
+
+ static {
+ try {
+ DISCONNECTED = MethodHandles.lookup().findVarHandle(AsyncSshHandler.class, "disconnected", boolean.class);
+ } catch (NoSuchFieldException | IllegalAccessException e) {
+ throw new ExceptionInInitializerError(e);
+ }
+ }
public static final String SUBSYSTEM = "netconf";
public static final NetconfSshClient DEFAULT_CLIENT;
static {
- final NetconfSshClient c = new NetconfClientBuilder().build();
+ final var c = new NetconfClientBuilder().build();
// Disable default timeouts from mina sshd
- c.getProperties().put(CoreModuleProperties.AUTH_TIMEOUT.getName(), "0");
- c.getProperties().put(CoreModuleProperties.IDLE_TIMEOUT.getName(), "0");
- c.getProperties().put(CoreModuleProperties.NIO2_READ_TIMEOUT.getName(), "0");
- c.getProperties().put(CoreModuleProperties.TCP_NODELAY.getName(), true);
+ final var zero = Duration.ofMillis(0);
+ CoreModuleProperties.AUTH_TIMEOUT.set(c, zero);
+ CoreModuleProperties.IDLE_TIMEOUT.set(c, zero);
+ CoreModuleProperties.NIO2_READ_TIMEOUT.set(c, zero);
+ CoreModuleProperties.TCP_NODELAY.set(c, true);
// TODO make configurable, or somehow reuse netty threadpool
c.setNioWorkers(SSH_DEFAULT_NIO_WORKERS);
DEFAULT_CLIENT = c;
}
- private final AtomicBoolean isDisconnected = new AtomicBoolean();
private final AuthenticationHandler authenticationHandler;
private final Future<?> negotiationFuture;
private final NetconfSshClient sshClient;
+ private final String name;
- private AsyncSshHandlerWriter sshWriteAsyncHandler;
+ // Initialized by connect()
+ @GuardedBy("this")
+ private ChannelPromise connectPromise;
- private NettyAwareChannelSubsystem channel;
+ private AsyncSshHandlerWriter sshWriteAsyncHandler;
+ private ChannelSubsystem channel;
private ClientSession session;
- private ChannelPromise connectPromise;
private FutureListener<Object> negotiationFutureListener;
- public AsyncSshHandler(final AuthenticationHandler authenticationHandler, final NetconfSshClient sshClient,
- final Future<?> negotiationFuture) {
+ private volatile boolean disconnected;
+
+ private AsyncSshHandler(final AuthenticationHandler authenticationHandler, final NetconfSshClient sshClient,
+ final @Nullable Future<?> negotiationFuture, final @Nullable String name) {
this.authenticationHandler = requireNonNull(authenticationHandler);
this.sshClient = requireNonNull(sshClient);
this.negotiationFuture = negotiationFuture;
+ this.name = name != null && !name.isBlank() ? name : "UNNAMED";
+ }
+
+ public AsyncSshHandler(final AuthenticationHandler authenticationHandler, final NetconfSshClient sshClient,
+ final @Nullable Future<?> negotiationFuture) {
+ this(authenticationHandler, sshClient, negotiationFuture, null);
}
/**
* @return {@code AsyncSshHandler}
*/
public static AsyncSshHandler createForNetconfSubsystem(final AuthenticationHandler authenticationHandler,
- final Future<?> negotiationFuture, final @Nullable NetconfSshClient sshClient) {
+ final Future<?> negotiationFuture, final @Nullable NetconfSshClient sshClient,
+ final @Nullable String name) {
return new AsyncSshHandler(authenticationHandler, sshClient != null ? sshClient : DEFAULT_CLIENT,
- negotiationFuture);
- }
-
- private synchronized void handleSshAuthenticated(final NettyAwareClientSession newSession,
- final ChannelHandlerContext ctx) {
- LOG.debug("SSH session authenticated on channel: {}, server version: {}", ctx.channel(),
- newSession.getServerVersion());
-
- try {
- channel = newSession.createSubsystemChannel(SUBSYSTEM, ctx);
- channel.setStreaming(ClientChannel.Streaming.Async);
- channel.open().addListener(future -> {
- if (future.isOpened()) {
- handleSshChanelOpened(ctx);
- } else {
- handleSshSetupFailure(ctx, future.getException());
- }
- });
- } catch (final IOException e) {
- handleSshSetupFailure(ctx, e);
- }
- }
-
- private synchronized void handleSshChanelOpened(final ChannelHandlerContext ctx) {
- LOG.trace("SSH subsystem channel opened successfully on channel: {}", ctx.channel());
-
- if (negotiationFuture == null) {
- connectPromise.setSuccess();
- }
-
- sshWriteAsyncHandler = new AsyncSshHandlerWriter(channel.getAsyncIn());
- ctx.fireChannelActive();
- channel.onClose(() -> disconnect(ctx, ctx.newPromise()));
- }
-
- private synchronized void handleSshSetupFailure(final ChannelHandlerContext ctx, final Throwable error) {
- LOG.warn("Unable to setup SSH connection on channel: {}", ctx.channel(), error);
-
- // If the promise is not yet done, we have failed with initial connect and set connectPromise to failure
- if (!connectPromise.isDone()) {
- connectPromise.setFailure(error);
- }
-
- disconnect(ctx, ctx.newPromise());
+ negotiationFuture, name);
}
@Override
@Override
public synchronized void connect(final ChannelHandlerContext ctx, final SocketAddress remoteAddress,
final SocketAddress localAddress, final ChannelPromise promise) throws IOException {
- LOG.debug("SSH session connecting on channel {}. promise: {}", ctx.channel(), promise);
+ LOG.debug("{}: SSH session connecting on channel {}. promise: {}", name, ctx.channel(), promise);
connectPromise = requireNonNull(promise);
if (negotiationFuture != null) {
negotiationFuture.addListener(negotiationFutureListener);
}
- LOG.debug("Starting SSH to {} on channel: {}", remoteAddress, ctx.channel());
+ LOG.debug("{}: Starting SSH to {} on channel: {}", name, remoteAddress, ctx.channel());
sshClient.connect(authenticationHandler.getUsername(), remoteAddress)
// FIXME: this is a blocking call, we should handle this with a concurrently-scheduled timeout. We do not
// have a Timer ready, so perhaps we should be using the event loop?
.addListener(future -> onConnectComplete(future, ctx));
}
- private void onConnectComplete(final ConnectFuture future, final ChannelHandlerContext ctx) {
- final var cause = future.getException();
+ private synchronized void onConnectComplete(final ConnectFuture connectFuture, final ChannelHandlerContext ctx) {
+ final var cause = connectFuture.getException();
if (cause != null) {
- handleSshSetupFailure(ctx, cause);
+ onOpenFailure(ctx, cause);
return;
}
- final var clientSession = future.getSession();
- LOG.trace("SSH session {} created on channel: {}", clientSession, ctx.channel());
+ final var clientSession = connectFuture.getSession();
+ LOG.trace("{}: SSH session {} created on channel: {}", name, clientSession, ctx.channel());
verify(clientSession instanceof NettyAwareClientSession, "Unexpected session %s", clientSession);
- onConnectComplete((NettyAwareClientSession) clientSession, ctx);
- }
- private synchronized void onConnectComplete(final NettyAwareClientSession clientSession,
- final ChannelHandlerContext ctx) {
- session = clientSession;
+ final var localSession = (NettyAwareClientSession) clientSession;
+ session = localSession;
final AuthFuture authFuture;
try {
- authFuture = authenticationHandler.authenticate(clientSession);
+ authFuture = authenticationHandler.authenticate(localSession);
} catch (final IOException e) {
- handleSshSetupFailure(ctx, e);
+ onOpenFailure(ctx, e);
return;
}
- authFuture.addListener(future -> onAuthComplete(future, clientSession, ctx));
+ authFuture.addListener(future -> onAuthComplete(future, localSession, ctx));
}
- private void onAuthComplete(final AuthFuture future, final NettyAwareClientSession clientSession,
+ private synchronized void onAuthComplete(final AuthFuture authFuture, final NettyAwareClientSession clientSession,
final ChannelHandlerContext ctx) {
- final var cause = future.getException();
+ final var cause = authFuture.getException();
+ if (cause != null) {
+ onOpenFailure(ctx, new AuthenticationFailedException("Authentication failed", cause));
+ return;
+ }
+ if (disconnected) {
+ LOG.debug("{}: Skipping SSH subsystem allocation, channel: {}", name, ctx.channel());
+ return;
+ }
+
+ LOG.debug("{}: SSH session authenticated on channel: {}, server version: {}", name, ctx.channel(),
+ clientSession.getServerVersion());
+
+ final OpenFuture openFuture;
+ try {
+ channel = clientSession.createSubsystemChannel(SUBSYSTEM, ctx);
+ channel.setStreaming(ClientChannel.Streaming.Async);
+ openFuture = channel.open();
+ } catch (final IOException e) {
+ onOpenFailure(ctx, e);
+ return;
+ }
+
+ openFuture.addListener(future -> ctx.executor().execute(() -> onOpenComplete(future, ctx)));
+ }
+
+ // This callback has to run on the channel's executor because it runs fireChannelActive(), which needs to be
+ // delivered synchronously. If we were to execute on some other thread we would end up delaying the event,
+ // potentially creating havoc in the pipeline.
+ private synchronized void onOpenComplete(final OpenFuture openFuture, final ChannelHandlerContext ctx) {
+ final var cause = openFuture.getException();
if (cause != null) {
- handleSshSetupFailure(ctx, new AuthenticationFailedException("Authentication failed", cause));
+ onOpenFailure(ctx, cause);
return;
}
+ if (disconnected) {
+ LOG.trace("{}: Skipping activation, channel: {}", name, ctx.channel());
+ return;
+ }
+
+ LOG.trace("{}: SSH subsystem channel opened successfully on channel: {}", name, ctx.channel());
+ if (negotiationFuture == null) {
+ connectPromise.setSuccess();
+ }
- handleSshAuthenticated(clientSession, ctx);
+ sshWriteAsyncHandler = new AsyncSshHandlerWriter(channel.getAsyncIn());
+ ctx.fireChannelActive();
+ channel.onClose(() -> disconnect(ctx, ctx.newPromise()));
+ }
+
+ @Holding("this")
+ private void onOpenFailure(final ChannelHandlerContext ctx, final Throwable cause) {
+ LOG.warn("{}: Unable to setup SSH connection on channel: {}", name, ctx.channel(), cause);
+
+ // If the promise is not yet done, we have failed with initial connect and set connectPromise to failure
+ if (!connectPromise.isDone()) {
+ connectPromise.setFailure(cause);
+ }
+
+ disconnect(ctx, ctx.newPromise());
}
@Override
@Override
public void disconnect(final ChannelHandlerContext ctx, final ChannelPromise promise) {
- if (isDisconnected.compareAndSet(false, true)) {
- safelyDisconnect(ctx, promise);
+ if (DISCONNECTED.compareAndSet(this, false, true)) {
+ ctx.executor().execute(() -> safelyDisconnect(ctx, promise));
}
}
+ // This method has the potential to interact with the channel pipeline, for example via fireChannelInactive(). These
+ // callbacks need to complete during execution of this method and therefore this method needs to be executing on
+ // the channel's executor.
@SuppressWarnings("checkstyle:IllegalCatch")
private synchronized void safelyDisconnect(final ChannelHandlerContext ctx, final ChannelPromise promise) {
- LOG.trace("Closing SSH session on channel: {} with connect promise in state: {}",
- ctx.channel(), connectPromise);
+ LOG.trace("{}: Closing SSH session on channel: {} with connect promise in state: {}", name, ctx.channel(),
+ connectPromise);
// If we have already succeeded and the session was dropped after,
// we need to fire inactive to notify reconnect logic
// Disconnect has to be closed after inactive channel event was fired, because it interferes with it
super.disconnect(ctx, ctx.newPromise());
} catch (final Exception e) {
- LOG.warn("Unable to cleanup all resources for channel: {}. Ignoring.", ctx.channel(), e);
+ LOG.warn("{}: Unable to cleanup all resources for channel: {}. Ignoring.", name, ctx.channel(), e);
}
if (channel != null) {
- //TODO: see if calling just close() is sufficient
- //channel.close(false);
- channel.close();
+ channel.close(false);
channel = null;
}
promise.setSuccess();
- LOG.debug("SSH session closed on channel: {}", ctx.channel());
+ LOG.debug("{}: SSH session closed on channel: {}", name, ctx.channel());
}
}
Code Review / netconf.git / blobdiff