Added Thread naming based on remote socket address (#738) (#753)

- Added ThreadNameProvider to set name based on Thread Class and remote socket address
- Added RemoteAddressProvider to abstract access to Remote Socket Address
- Set Reader Thread name in TransportImpl
- Set SFTP PacketReader Thread name in SFTPEngine
- Set KeepAlive Thread name in SSHClient

Co-authored-by: Jeroen van Erp <jeroen@hierynomus.com>

Author: exceptionfactory

src/main/java/com/hierynomus/sshj/common/RemoteAddressProvider.java

@@ -0,0 +1,27 @@
+/*
+ * Copyright (C)2009 - SSHJ Contributors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.hierynomus.sshj.common;
+
+import java.net.InetSocketAddress;
+
+public interface RemoteAddressProvider {
+    /**
+     * Get Remote Socket Address associated with transport connection
+     *
+     * @return Remote Socket Address or null when not connected
+     */
+    InetSocketAddress getRemoteSocketAddress();
+}

src/main/java/com/hierynomus/sshj/common/ThreadNameProvider.java

@@ -0,0 +1,35 @@
+/*
+ * Copyright (C)2009 - SSHJ Contributors
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package com.hierynomus.sshj.common;
+
+import java.net.InetSocketAddress;
+
+public class ThreadNameProvider {
+    private static final String DISCONNECTED = "DISCONNECTED";
+
+    /**
+     * Set Thread Name prefixed with sshj followed by class and remote address when connected
+     *
+     * @param thread Class of Thread being named
+     * @param remoteAddressProvider Remote Address Provider associated with Thread
+     */
+    public static void setThreadName(final Thread thread, final RemoteAddressProvider remoteAddressProvider) {
+        final InetSocketAddress remoteSocketAddress = remoteAddressProvider.getRemoteSocketAddress();
+        final String address = remoteSocketAddress == null ? DISCONNECTED : remoteSocketAddress.toString();
+        final String threadName = String.format("sshj-%s-%s", thread.getClass().getSimpleName(), address);
+        thread.setName(threadName);
+    }
+}

src/main/java/net/schmizz/keepalive/Heartbeater.java

@@ -24,7 +24,7 @@ final class Heartbeater
         extends KeepAlive {
 
     Heartbeater(ConnectionImpl conn) {
-        super(conn, "heartbeater");
+        super(conn, "sshj-Heartbeater");
     }
 
     @Override

src/main/java/net/schmizz/keepalive/KeepAliveRunner.java

@@ -37,7 +37,7 @@ public class KeepAliveRunner extends KeepAlive {
             new LinkedList<Promise<SSHPacket, ConnectionException>>();
 
     KeepAliveRunner(ConnectionImpl conn) {
-        super(conn, "keep-alive");
+        super(conn, "sshj-KeepAliveRunner");
     }
 
     synchronized public int getMaxAliveCount() {

src/main/java/net/schmizz/sshj/SSHClient.java

@@ -16,6 +16,7 @@
 package net.schmizz.sshj;
 
 import net.schmizz.keepalive.KeepAlive;
+import com.hierynomus.sshj.common.ThreadNameProvider;
 import net.schmizz.sshj.common.*;
 import net.schmizz.sshj.connection.Connection;
 import net.schmizz.sshj.connection.ConnectionException;
@@ -56,6 +57,7 @@
 import java.io.Closeable;
 import java.io.File;
 import java.io.IOException;
+import java.net.InetSocketAddress;
 import java.net.ServerSocket;
 import java.nio.charset.Charset;
 import java.security.KeyPair;
@@ -443,6 +445,16 @@ public Connection getConnection() {
         return conn;
     }
 
+    /**
+     * Get Remote Socket Address from Transport
+     *
+     * @return Remote Socket Address or null when not connected
+     */
+    @Override
+    public InetSocketAddress getRemoteSocketAddress() {
+        return trans.getRemoteSocketAddress();
+    }
+
     /**
      * Returns the character set used to communicate with the remote machine for certain strings (like paths).
      *
@@ -795,6 +807,7 @@ protected void onConnect()
         trans.init(getRemoteHostname(), getRemotePort(), getInputStream(), getOutputStream());
         final KeepAlive keepAliveThread = conn.getKeepAlive();
         if (keepAliveThread.isEnabled()) {
+            ThreadNameProvider.setThreadName(conn.getKeepAlive(), trans);
             keepAliveThread.start();
         }
         doKex();

src/main/java/net/schmizz/sshj/connection/channel/direct/SessionFactory.java

@@ -15,10 +15,11 @@
  */
 package net.schmizz.sshj.connection.channel.direct;
 
+import com.hierynomus.sshj.common.RemoteAddressProvider;
 import net.schmizz.sshj.common.SSHException;
 
 /** A factory interface for creating SSH {@link Session session channels}. */
-public interface SessionFactory {
+public interface SessionFactory extends RemoteAddressProvider {
 
     /**
      * Opens a {@code session} channel. The returned {@link Session} instance allows {@link Session#exec(String)
@@ -27,7 +28,7 @@ public interface SessionFactory {
      *
      * @return the opened {@code session} channel
      *
-     * @throws SSHException
+     * @throws SSHException Thrown on session initialization failures
      * @see Session
      */
     Session startSession()

src/main/java/net/schmizz/sshj/sftp/PacketReader.java

@@ -41,7 +41,7 @@ public PacketReader(SFTPEngine engine) {
         this.engine = engine;
         log = engine.getLoggerFactory().getLogger(getClass());
         this.in = engine.getSubsystem().getInputStream();
-        setName("sftp reader");
+        setName("sshj-PacketReader");
         setDaemon(true);
     }
 

src/main/java/net/schmizz/sshj/sftp/SFTPEngine.java

@@ -15,6 +15,7 @@
  */
 package net.schmizz.sshj.sftp;
 
+import com.hierynomus.sshj.common.ThreadNameProvider;
 import net.schmizz.concurrent.Promise;
 import net.schmizz.sshj.common.IOUtils;
 import net.schmizz.sshj.common.LoggerFactory;
@@ -68,6 +69,7 @@ public SFTPEngine(SessionFactory ssh, String pathSep)
         sub = session.startSubsystem("sftp");
         out = sub.getOutputStream();
         reader = new PacketReader(this);
+        ThreadNameProvider.setThreadName(reader, ssh);
         pathHelper = new PathHelper(new PathHelper.Canonicalizer() {
             @Override
             public String canonicalize(String path)

src/main/java/net/schmizz/sshj/transport/Reader.java

@@ -29,7 +29,7 @@ public final class Reader
     public Reader(TransportImpl trans) {
         this.trans = trans;
         log = trans.getConfig().getLoggerFactory().getLogger(getClass());
-        setName("reader");
+        setName("sshj-Reader");
         setDaemon(true);
     }
 

src/main/java/net/schmizz/sshj/transport/Transport.java

@@ -15,6 +15,7 @@
  */
 package net.schmizz.sshj.transport;
 
+import com.hierynomus.sshj.common.RemoteAddressProvider;
 import com.hierynomus.sshj.key.KeyAlgorithm;
 import net.schmizz.sshj.Config;
 import net.schmizz.sshj.Service;
@@ -31,7 +32,7 @@
 
 /** Transport layer of the SSH protocol. */
 public interface Transport
-        extends SSHPacketHandler {
+        extends SSHPacketHandler, RemoteAddressProvider {
 
     /**
      * Sets the host information and the streams to be used by this transport. Identification information is exchanged
@@ -208,7 +209,7 @@ long write(SSHPacket payload)
     /**
      * Specify a {@code listener} that will be notified upon disconnection.
      *
-     * @param listener
+     * @param listener Disconnect Listener to be configured
      */
     void setDisconnectListener(DisconnectListener listener);
 

src/main/java/net/schmizz/sshj/transport/TransportImpl.java

@@ -15,14 +15,14 @@
  */
 package net.schmizz.sshj.transport;
 
+import com.hierynomus.sshj.common.ThreadNameProvider;
 import com.hierynomus.sshj.key.KeyAlgorithm;
 import com.hierynomus.sshj.key.KeyAlgorithms;
 import com.hierynomus.sshj.transport.IdentificationStringParser;
 import net.schmizz.concurrent.ErrorDeliveryUtil;
 import net.schmizz.concurrent.Event;
 import net.schmizz.sshj.AbstractService;
 import net.schmizz.sshj.Config;
-import net.schmizz.sshj.SSHClient;
 import net.schmizz.sshj.Service;
 import net.schmizz.sshj.common.*;
 import net.schmizz.sshj.transport.verification.AlgorithmsVerifier;
@@ -32,6 +32,7 @@
 import java.io.IOException;
 import java.io.InputStream;
 import java.io.OutputStream;
+import java.net.InetSocketAddress;
 import java.util.List;
 import java.util.concurrent.TimeUnit;
 import java.util.concurrent.locks.ReentrantLock;
@@ -128,8 +129,8 @@ static final class ConnInfo {
     public TransportImpl(Config config) {
         this.config = config;
         this.loggerFactory = config.getLoggerFactory();
-        this.serviceAccept = new Event<TransportException>("service accept", TransportException.chainer, loggerFactory);
-        this.close = new Event<TransportException>("transport close", TransportException.chainer, loggerFactory);
+        this.serviceAccept = new Event<>("service accept", TransportException.chainer, loggerFactory);
+        this.close = new Event<>("transport close", TransportException.chainer, loggerFactory);
         this.nullService = new NullService(this);
         this.service = nullService;
         this.log = loggerFactory.getLogger(getClass());
@@ -163,9 +164,20 @@ public void init(String remoteHost, int remotePort, InputStream in, OutputStream
             throw new TransportException(e);
         }
 
+        ThreadNameProvider.setThreadName(reader, this);
         reader.start();
     }
 
+    /**
+     * Get Remote Socket Address using Connection Information
+     *
+     * @return Remote Socket Address or null when not connected
+     */
+    @Override
+    public InetSocketAddress getRemoteSocketAddress() {
+        return connInfo == null ? null : new InetSocketAddress(getRemoteHost(), getRemotePort());
+    }
+
     /**
      * TransportImpl implements its own default DisconnectListener.
      */
@@ -209,7 +221,7 @@ private void sendClientIdent() throws IOException {
      *
      * @param buffer The buffer to read from.
      * @return empty string if full ident string has not yet been received
-     * @throws IOException
+     * @throws IOException Thrown when protocol version is not supported
      */
     private String readIdentification(Buffer.PlainBuffer buffer)
             throws IOException {
@@ -542,7 +554,7 @@ private void gotServiceAccept()
      * Got an SSH_MSG_UNIMPLEMENTED, so lets see where we're at and act accordingly.
      *
      * @param packet The 'unimplemented' packet received
-     * @throws TransportException
+     * @throws TransportException Thrown when key exchange is ongoing
      */
     private void gotUnimplemented(SSHPacket packet)
             throws SSHException {