From 19263590a7be1c1184fc4c941fd5ed349da9e646 Mon Sep 17 00:00:00 2001 From: Alex Johnson Date: Tue, 21 Jan 2020 09:51:16 -0500 Subject: [PATCH 1/2] Changes Ssh client autoreconnect method to null client and start with new client on next attempt. Saw issues specifically with Polycom not reconnecting unless we spin up a new client. --- .../Pepperdash Core/Comm/GenericSshClient.cs | 41 +++++++++++-------- 1 file changed, 25 insertions(+), 16 deletions(-) diff --git a/Pepperdash Core/Pepperdash Core/Comm/GenericSshClient.cs b/Pepperdash Core/Pepperdash Core/Comm/GenericSshClient.cs index 8a6e9fd..4c06934 100644 --- a/Pepperdash Core/Pepperdash Core/Comm/GenericSshClient.cs +++ b/Pepperdash Core/Pepperdash Core/Comm/GenericSshClient.cs @@ -207,25 +207,24 @@ namespace PepperDash.Core return; } - // This handles both password and keyboard-interactive (like on OS-X, 'nixes) - KeyboardInteractiveAuthenticationMethod kauth = new KeyboardInteractiveAuthenticationMethod(Username); - kauth.AuthenticationPrompt += new EventHandler(kauth_AuthenticationPrompt); - PasswordAuthenticationMethod pauth = new PasswordAuthenticationMethod(Username, Password); - if (Client != null) { Debug.Console(1, this, "Cleaning up disconnected client"); Client.ErrorOccurred -= Client_ErrorOccurred; KillStream(); + Client.Disconnect(); } - - Debug.Console(1, this, "Creating new SshClient"); - ConnectionInfo connectionInfo = new ConnectionInfo(Hostname, Port, Username, pauth, kauth); - - if (Client == null) + else { + // This handles both password and keyboard-interactive (like on OS-X, 'nixes) + KeyboardInteractiveAuthenticationMethod kauth = new KeyboardInteractiveAuthenticationMethod(Username); + kauth.AuthenticationPrompt += new EventHandler(kauth_AuthenticationPrompt); + PasswordAuthenticationMethod pauth = new PasswordAuthenticationMethod(Username, Password); + Debug.Console(1, this, "Creating new SshClient"); + ConnectionInfo connectionInfo = new ConnectionInfo(Hostname, Port, Username, pauth, kauth); Client = new SshClient(connectionInfo); } + Client.ErrorOccurred -= Client_ErrorOccurred; Client.ErrorOccurred += Client_ErrorOccurred; @@ -298,9 +297,13 @@ namespace PepperDash.Core /// void HandleConnectionFailure() { - if (Client != null) - Client.Disconnect(); + if (Client != null) + { + Client.Disconnect(); + Client = null; + } KillStream(); + Debug.Console(2, this, "Client nulled due to connection failure. AutoReconnect: {0}, ConnectEnabled: {1}", AutoReconnect, ConnectEnabled); if (AutoReconnect && ConnectEnabled) { @@ -418,8 +421,11 @@ namespace PepperDash.Core { try { - TheStream.Write(text); - TheStream.Flush(); + if (Client != null) + { + TheStream.Write(text); + TheStream.Flush(); + } } catch { @@ -433,8 +439,11 @@ namespace PepperDash.Core { try { - TheStream.Write(bytes, 0, bytes.Length); - TheStream.Flush(); + if (Client != null) + { + TheStream.Write(bytes, 0, bytes.Length); + TheStream.Flush(); + } } catch { From 5576fa349d69f7386e765e0ff35a1a17044082da Mon Sep 17 00:00:00 2001 From: Neil Dorin Date: Fri, 24 Jan 2020 15:29:54 -0700 Subject: [PATCH 2/2] Cleans up the disconnect logic and adds KillClient() to better control process of disposing of resources before reconnect attempts. Adds XML help for propeties and methods --- .../Pepperdash Core/Comm/GenericSshClient.cs | 225 +++++++++--------- 1 file changed, 118 insertions(+), 107 deletions(-) diff --git a/Pepperdash Core/Pepperdash Core/Comm/GenericSshClient.cs b/Pepperdash Core/Pepperdash Core/Comm/GenericSshClient.cs index 4c06934..46892c7 100644 --- a/Pepperdash Core/Pepperdash Core/Comm/GenericSshClient.cs +++ b/Pepperdash Core/Pepperdash Core/Comm/GenericSshClient.cs @@ -175,7 +175,6 @@ namespace PepperDash.Core { Debug.Console(1, this, "Program stopping. Closing connection"); Disconnect(); - //Client.Dispose(); } } } @@ -184,87 +183,86 @@ namespace PepperDash.Core /// Connect to the server, using the provided properties. /// public void Connect() - { - ConnectEnabled = true; - Debug.Console(1, this, "attempting connect"); - - // Cancel reconnect if running. - if (ReconnectTimer != null) - { - ReconnectTimer.Stop(); - ReconnectTimer = null; - } + { + ConnectEnabled = true; + Debug.Console(1, this, "attempting connect"); - // Don't try to connect if already - if (IsConnected) - return; - - // Don't go unless everything is here - if (string.IsNullOrEmpty(Hostname) || Port < 1 || Port > 65535 - || Username == null || Password == null) - { - Debug.Console(1, this, "Connect failed. Check hostname, port, username and password are set or not null"); - return; - } + // Cancel reconnect if running. + if (ReconnectTimer != null) + { + ReconnectTimer.Stop(); + ReconnectTimer = null; + } - if (Client != null) - { - Debug.Console(1, this, "Cleaning up disconnected client"); - Client.ErrorOccurred -= Client_ErrorOccurred; - KillStream(); - Client.Disconnect(); - } - else - { - // This handles both password and keyboard-interactive (like on OS-X, 'nixes) - KeyboardInteractiveAuthenticationMethod kauth = new KeyboardInteractiveAuthenticationMethod(Username); - kauth.AuthenticationPrompt += new EventHandler(kauth_AuthenticationPrompt); - PasswordAuthenticationMethod pauth = new PasswordAuthenticationMethod(Username, Password); - Debug.Console(1, this, "Creating new SshClient"); - ConnectionInfo connectionInfo = new ConnectionInfo(Hostname, Port, Username, pauth, kauth); - Client = new SshClient(connectionInfo); - } + // Don't try to connect if already + if (IsConnected) + return; - Client.ErrorOccurred -= Client_ErrorOccurred; - Client.ErrorOccurred += Client_ErrorOccurred; + // Don't go unless everything is here + if (string.IsNullOrEmpty(Hostname) || Port < 1 || Port > 65535 + || Username == null || Password == null) + { + Debug.Console(1, this, "Connect failed. Check hostname, port, username and password are set or not null"); + return; + } - //You can do it! - ClientStatus = SocketStatus.SOCKET_STATUS_WAITING; - try - { - Client.Connect(); - TheStream = Client.CreateShellStream("PDTShell", 100, 80, 100, 200, 65534); - TheStream.DataReceived += Stream_DataReceived; - //TheStream.ErrorOccurred += TheStream_ErrorOccurred; - Debug.Console(1, this, "Connected"); - ClientStatus = SocketStatus.SOCKET_STATUS_CONNECTED; - return; // Success will not pass here - } - catch (SshConnectionException e) - { - var ie = e.InnerException; // The details are inside!! - if (ie is SocketException) - Debug.Console(1, this, "'{0}' CONNECTION failure: Cannot reach host, ({1})", Key, ie.GetType()); - else if (ie is System.Net.Sockets.SocketException) - Debug.Console(1, this, "'{0}' Connection failure: Cannot reach host '{1}' on port {2}, ({3})", - Key, Hostname, Port, ie.GetType()); - else if (ie is SshAuthenticationException) - { - Debug.Console(1, this, "Authentication failure for username '{0}', ({1})", - Username, ie.GetType()); - } - else - Debug.Console(1, this, "Error on connect:\r({0})", e); - } - catch (Exception e) - { - Debug.Console(1, this, "Unhandled exception on connect:\r({0})", e); - } - - // Sucess will not make it this far - ClientStatus = SocketStatus.SOCKET_STATUS_CONNECT_FAILED; - HandleConnectionFailure(); - } + // Cleanup the old client if it already exists + if (Client != null) + { + Debug.Console(1, this, "Cleaning up disconnected client"); + Client.ErrorOccurred -= Client_ErrorOccurred; + KillClient(SocketStatus.SOCKET_STATUS_BROKEN_LOCALLY); + } + + // This handles both password and keyboard-interactive (like on OS-X, 'nixes) + KeyboardInteractiveAuthenticationMethod kauth = new KeyboardInteractiveAuthenticationMethod(Username); + kauth.AuthenticationPrompt += new EventHandler(kauth_AuthenticationPrompt); + PasswordAuthenticationMethod pauth = new PasswordAuthenticationMethod(Username, Password); + + Debug.Console(1, this, "Creating new SshClient"); + ConnectionInfo connectionInfo = new ConnectionInfo(Hostname, Port, Username, pauth, kauth); + Client = new SshClient(connectionInfo); + + Client.ErrorOccurred -= Client_ErrorOccurred; + Client.ErrorOccurred += Client_ErrorOccurred; + + //Attempt to connect + ClientStatus = SocketStatus.SOCKET_STATUS_WAITING; + try + { + Client.Connect(); + TheStream = Client.CreateShellStream("PDTShell", 100, 80, 100, 200, 65534); + TheStream.DataReceived += Stream_DataReceived; + //TheStream.ErrorOccurred += TheStream_ErrorOccurred; + Debug.Console(1, this, "Connected"); + ClientStatus = SocketStatus.SOCKET_STATUS_CONNECTED; + return; // Success will not pass here + } + catch (SshConnectionException e) + { + var ie = e.InnerException; // The details are inside!! + if (ie is SocketException) + Debug.Console(1, this, "'{0}' CONNECTION failure: Cannot reach host, ({1})", Key, ie.GetType()); + else if (ie is System.Net.Sockets.SocketException) + Debug.Console(1, this, "'{0}' Connection failure: Cannot reach host '{1}' on port {2}, ({3})", + Key, Hostname, Port, ie.GetType()); + else if (ie is SshAuthenticationException) + { + Debug.Console(1, this, "Authentication failure for username '{0}', ({1})", + Username, ie.GetType()); + } + else + Debug.Console(1, this, "Error on connect:\r({0})", e); + } + catch (Exception e) + { + Debug.Console(1, this, "Unhandled exception on connect:\r({0})", e); + } + + // Sucess will not make it this far + ClientStatus = SocketStatus.SOCKET_STATUS_CONNECT_FAILED; + HandleConnectionFailure(); + } @@ -281,28 +279,32 @@ namespace PepperDash.Core ReconnectTimer = null; } - KillStream(); + KillClient(SocketStatus.SOCKET_STATUS_BROKEN_LOCALLY); + } + + /// + /// Kills the stream, cleans up the client and sets it to null + /// + private void KillClient(SocketStatus status) + { + KillStream(); if (Client != null) { Client.Disconnect(); Client = null; - ClientStatus = SocketStatus.SOCKET_STATUS_BROKEN_LOCALLY; + ClientStatus = status; Debug.Console(1, this, "Disconnected"); } - } + } /// /// Anything to do with reestablishing connection on failures /// void HandleConnectionFailure() { - if (Client != null) - { - Client.Disconnect(); - Client = null; - } - KillStream(); + KillClient(SocketStatus.SOCKET_STATUS_CONNECT_FAILED); + Debug.Console(2, this, "Client nulled due to connection failure. AutoReconnect: {0}, ConnectEnabled: {1}", AutoReconnect, ConnectEnabled); if (AutoReconnect && ConnectEnabled) @@ -327,24 +329,20 @@ namespace PepperDash.Core } } + /// + /// Kills the stream + /// void KillStream() { if (TheStream != null) { TheStream.DataReceived -= Stream_DataReceived; - //TheStream.ErrorOccurred -= TheStream_ErrorOccurred; TheStream.Close(); TheStream.Dispose(); TheStream = null; } } - //bool PropertiesHaveChanged() - //{ - // return Hostname != PreviousHostname || Port != PreviousPort - // || Username != PreviousUsername || Password != PreviousPassword; - //} - /// /// Handles the keyboard interactive authentication, should it be required. /// @@ -375,17 +373,6 @@ namespace PepperDash.Core } } - ///// - ///// Handler for errors on the stream... - ///// - ///// - ///// - //void TheStream_ErrorOccurred(object sender, ExceptionEventArgs e) - //{ - // Debug.Console(1, this, "Unhandled SSH STREAM error: {0}", e.Exception); - // ClientStatus = SocketStatus.SOCKET_STATUS_BROKEN_REMOTELY; - // HandleConnectionFailure(); - //} /// /// Error event handler for client events - disconnect, etc. Will forward those events via ConnectionChange @@ -414,7 +401,7 @@ namespace PepperDash.Core #region IBasicCommunication Members /// - /// + /// Sends text to the server /// /// public void SendText(string text) @@ -435,6 +422,10 @@ namespace PepperDash.Core } } + /// + /// Sends Bytes to the server + /// + /// public void SendBytes(byte[] bytes) { try @@ -463,16 +454,36 @@ namespace PepperDash.Core /// public class SshConnectionChangeEventArgs : EventArgs { + /// + /// Connection State + /// public bool IsConnected { get; private set; } + /// + /// Connection Status represented as a ushort + /// public ushort UIsConnected { get { return (ushort)(Client.IsConnected ? 1 : 0); } } + /// + /// The client + /// public GenericSshClient Client { get; private set; } + + /// + /// Socket Status as represented by + /// public ushort Status { get { return Client.UStatus; } } - // S+ Constructor + /// + /// S+ Constructor + /// public SshConnectionChangeEventArgs() { } + /// + /// EventArgs class + /// + /// Connection State + /// The Client public SshConnectionChangeEventArgs(bool isConnected, GenericSshClient client) { IsConnected = isConnected;