diff --git a/src/PepperDash.Core/Comm/GenericSshClient.cs b/src/PepperDash.Core/Comm/GenericSshClient.cs index 8ea5138e..c24ea829 100644 --- a/src/PepperDash.Core/Comm/GenericSshClient.cs +++ b/src/PepperDash.Core/Comm/GenericSshClient.cs @@ -1,4 +1,5 @@ using System; +using System.Collections.Generic; using System.Text; using System.Threading; using Crestron.SimplSharp; @@ -11,11 +12,12 @@ using Renci.SshNet.Common; namespace PepperDash.Core { /// - /// + /// SSH Client /// public class GenericSshClient : Device, ISocketStatusWithStreamDebugging, IAutoReconnect { private const string SPlusKey = "Uninitialized SshClient"; + /// /// Object to enable stream debugging /// @@ -36,11 +38,6 @@ namespace PepperDash.Core /// public event EventHandler ConnectionChange; - /// - ///// - ///// - //public event GenericSocketStatusChangeEventDelegate SocketStatusChange; - /// /// Gets or sets the Hostname /// @@ -67,7 +64,7 @@ namespace PepperDash.Core public bool IsConnected { // returns false if no client or not connected - get { return Client != null && ClientStatus == SocketStatus.SOCKET_STATUS_CONNECTED; } + get { return client != null && ClientStatus == SocketStatus.SOCKET_STATUS_CONNECTED; } } /// @@ -83,16 +80,26 @@ namespace PepperDash.Core /// public SocketStatus ClientStatus { - get { return _ClientStatus; } + get { lock (_stateLock) { return _ClientStatus; } } private set { - if (_ClientStatus == value) - return; - _ClientStatus = value; - OnConnectionChange(); + bool shouldFireEvent = false; + lock (_stateLock) + { + if (_ClientStatus != value) + { + _ClientStatus = value; + shouldFireEvent = true; + } + } + // Fire event outside lock to avoid deadlock + if (shouldFireEvent) + OnConnectionChange(); } } - SocketStatus _ClientStatus; + + private SocketStatus _ClientStatus; + private bool _ConnectEnabled; /// /// Contains the familiar Simpl analog status values. This drives the ConnectionChange event @@ -100,7 +107,7 @@ namespace PepperDash.Core /// public ushort UStatus { - get { return (ushort)_ClientStatus; } + get { lock (_stateLock) { return (ushort)_ClientStatus; } } } /// @@ -111,7 +118,11 @@ namespace PepperDash.Core /// /// Will be set and unset by connect and disconnect only /// - public bool ConnectEnabled { get; private set; } + public bool ConnectEnabled + { + get { lock (_stateLock) { return _ConnectEnabled; } } + private set { lock (_stateLock) { _ConnectEnabled = value; } } + } /// /// S+ helper for AutoReconnect @@ -127,17 +138,25 @@ namespace PepperDash.Core /// public int AutoReconnectIntervalMs { get; set; } - SshClient Client; + private SshClient client; - ShellStream TheStream; + private ShellStream shellStream; - CTimer ReconnectTimer; + private readonly Timer reconnectTimer; //Lock object to prevent simulatneous connect/disconnect operations //private CCriticalSection connectLock = new CCriticalSection(); - private SemaphoreSlim connectLock = new SemaphoreSlim(1); + private readonly SemaphoreSlim connectLock = new SemaphoreSlim(1); - private bool DisconnectLogged = false; + // Thread-safety lock for state changes + private readonly object _stateLock = new object(); + + private bool disconnectLogged = false; + + /// + /// When true, turns off echo for the SSH session + /// + public bool DisableEcho { get; set; } /// /// Typical constructor. @@ -154,13 +173,13 @@ namespace PepperDash.Core Password = password; AutoReconnectIntervalMs = 5000; - ReconnectTimer = new CTimer(o => + reconnectTimer = new Timer(o => { - if (ConnectEnabled) + if (ConnectEnabled) // Now thread-safe property access { Connect(); } - }, System.Threading.Timeout.Infinite); + }, null, System.Threading.Timeout.Infinite, System.Threading.Timeout.Infinite); } /// @@ -172,23 +191,23 @@ namespace PepperDash.Core CrestronEnvironment.ProgramStatusEventHandler += new ProgramStatusEventHandler(CrestronEnvironment_ProgramStatusEventHandler); AutoReconnectIntervalMs = 5000; - ReconnectTimer = new CTimer(o => + reconnectTimer = new Timer(o => { - if (ConnectEnabled) + if (ConnectEnabled) // Now thread-safe property access { Connect(); } - }, System.Threading.Timeout.Infinite); + }, null, System.Threading.Timeout.Infinite, System.Threading.Timeout.Infinite); } /// /// Handles closing this up when the program shuts down /// - void CrestronEnvironment_ProgramStatusEventHandler(eProgramStatusEventType programEventType) + private void CrestronEnvironment_ProgramStatusEventHandler(eProgramStatusEventType programEventType) { if (programEventType == eProgramStatusEventType.Stopping) { - if (Client != null) + if (client != null) { this.LogDebug("Program stopping. Closing connection"); Disconnect(); @@ -223,10 +242,10 @@ namespace PepperDash.Core this.LogDebug("Attempting connect"); // Cancel reconnect if running. - ReconnectTimer?.Stop(); + StopReconnectTimer(); // Cleanup the old client if it already exists - if (Client != null) + if (client != null) { this.LogDebug("Cleaning up disconnected client"); KillClient(SocketStatus.SOCKET_STATUS_BROKEN_LOCALLY); @@ -239,29 +258,37 @@ namespace PepperDash.Core this.LogDebug("Creating new SshClient"); ConnectionInfo connectionInfo = new ConnectionInfo(Hostname, Port, Username, pauth, kauth); - Client = new SshClient(connectionInfo); - Client.ErrorOccurred += Client_ErrorOccurred; + client = new SshClient(connectionInfo); + client.ErrorOccurred += Client_ErrorOccurred; //Attempt to connect ClientStatus = SocketStatus.SOCKET_STATUS_WAITING; try { - Client.Connect(); - TheStream = Client.CreateShellStream("PDTShell", 0, 0, 0, 0, 65534); - if (TheStream.DataAvailable) + client.Connect(); + + var modes = new Dictionary(); + + if (DisableEcho) + { + modes.Add(TerminalModes.ECHO, 0); + } + + shellStream = client.CreateShellStream("PDTShell", 0, 0, 0, 0, 65534, modes); + if (shellStream.DataAvailable) { // empty the buffer if there is data - string str = TheStream.Read(); + string str = shellStream.Read(); } - TheStream.DataReceived += Stream_DataReceived; + shellStream.DataReceived += Stream_DataReceived; this.LogInformation("Connected"); ClientStatus = SocketStatus.SOCKET_STATUS_CONNECTED; - DisconnectLogged = false; + disconnectLogged = false; } catch (SshConnectionException e) { var ie = e.InnerException; // The details are inside!! - var errorLogLevel = DisconnectLogged == true ? Debug.ErrorLogLevel.None : Debug.ErrorLogLevel.Error; + var errorLogLevel = disconnectLogged == true ? Debug.ErrorLogLevel.None : Debug.ErrorLogLevel.Error; if (ie is SocketException) { @@ -286,37 +313,37 @@ namespace PepperDash.Core this.LogVerbose(ie, "Exception details: "); } - DisconnectLogged = true; + disconnectLogged = true; KillClient(SocketStatus.SOCKET_STATUS_CONNECT_FAILED); if (AutoReconnect) { this.LogDebug("Checking autoreconnect: {autoReconnect}, {autoReconnectInterval}ms", AutoReconnect, AutoReconnectIntervalMs); - ReconnectTimer.Reset(AutoReconnectIntervalMs); + StartReconnectTimer(); } } catch (SshOperationTimeoutException ex) { this.LogWarning("Connection attempt timed out: {message}", ex.Message); - DisconnectLogged = true; + disconnectLogged = true; KillClient(SocketStatus.SOCKET_STATUS_CONNECT_FAILED); if (AutoReconnect) { this.LogDebug("Checking autoreconnect: {0}, {1}ms", AutoReconnect, AutoReconnectIntervalMs); - ReconnectTimer.Reset(AutoReconnectIntervalMs); + StartReconnectTimer(); } } catch (Exception e) { - var errorLogLevel = DisconnectLogged == true ? Debug.ErrorLogLevel.None : Debug.ErrorLogLevel.Error; + var errorLogLevel = disconnectLogged == true ? Debug.ErrorLogLevel.None : Debug.ErrorLogLevel.Error; this.LogError("Unhandled exception on connect: {error}", e.Message); this.LogVerbose(e, "Exception details: "); - DisconnectLogged = true; + disconnectLogged = true; KillClient(SocketStatus.SOCKET_STATUS_CONNECT_FAILED); if (AutoReconnect) { this.LogDebug("Checking autoreconnect: {0}, {1}ms", AutoReconnect, AutoReconnectIntervalMs); - ReconnectTimer.Reset(AutoReconnectIntervalMs); + StartReconnectTimer(); } } } @@ -334,11 +361,7 @@ namespace PepperDash.Core { ConnectEnabled = false; // Stop trying reconnects, if we are - if (ReconnectTimer != null) - { - ReconnectTimer.Stop(); - // ReconnectTimer = null; - } + StopReconnectTimer(); KillClient(SocketStatus.SOCKET_STATUS_BROKEN_LOCALLY); } @@ -352,12 +375,12 @@ namespace PepperDash.Core try { - if (Client != null) + if (client != null) { - Client.ErrorOccurred -= Client_ErrorOccurred; - Client.Disconnect(); - Client.Dispose(); - Client = null; + client.ErrorOccurred -= Client_ErrorOccurred; + client.Disconnect(); + client.Dispose(); + client = null; ClientStatus = status; this.LogDebug("Disconnected"); } @@ -371,16 +394,16 @@ namespace PepperDash.Core /// /// Kills the stream /// - void KillStream() + private void KillStream() { try { - if (TheStream != null) + if (shellStream != null) { - TheStream.DataReceived -= Stream_DataReceived; - TheStream.Close(); - TheStream.Dispose(); - TheStream = null; + shellStream.DataReceived -= Stream_DataReceived; + shellStream.Close(); + shellStream.Dispose(); + shellStream = null; this.LogDebug("Disconnected stream"); } } @@ -393,7 +416,7 @@ namespace PepperDash.Core /// /// Handles the keyboard interactive authentication, should it be required. /// - void kauth_AuthenticationPrompt(object sender, AuthenticationPromptEventArgs e) + private void kauth_AuthenticationPrompt(object sender, AuthenticationPromptEventArgs e) { foreach (AuthenticationPrompt prompt in e.Prompts) if (prompt.Request.IndexOf("Password:", StringComparison.InvariantCultureIgnoreCase) != -1) @@ -403,7 +426,7 @@ namespace PepperDash.Core /// /// Handler for data receive on ShellStream. Passes data across to queue for line parsing. /// - void Stream_DataReceived(object sender, ShellDataEventArgs e) + private void Stream_DataReceived(object sender, ShellDataEventArgs e) { if (((ShellStream)sender).Length <= 0L) { @@ -435,7 +458,7 @@ namespace PepperDash.Core /// Error event handler for client events - disconnect, etc. Will forward those events via ConnectionChange /// event /// - void Client_ErrorOccurred(object sender, ExceptionEventArgs e) + private void Client_ErrorOccurred(object sender, ExceptionEventArgs e) { CrestronInvoke.BeginInvoke(o => { @@ -455,7 +478,7 @@ namespace PepperDash.Core if (AutoReconnect && ConnectEnabled) { this.LogDebug("Checking autoreconnect: {0}, {1}ms", AutoReconnect, AutoReconnectIntervalMs); - ReconnectTimer.Reset(AutoReconnectIntervalMs); + StartReconnectTimer(); } }); } @@ -463,7 +486,7 @@ namespace PepperDash.Core /// /// Helper for ConnectionChange event /// - void OnConnectionChange() + private void OnConnectionChange() { ConnectionChange?.Invoke(this, new GenericSocketStatusChageEventArgs(this)); } @@ -478,12 +501,12 @@ namespace PepperDash.Core { try { - if (Client != null && TheStream != null && IsConnected) + if (client != null && shellStream != null && IsConnected) { this.PrintSentText(text); - TheStream.Write(text); - TheStream.Flush(); + shellStream.Write(text); + shellStream.Flush(); } else { @@ -495,7 +518,7 @@ namespace PepperDash.Core this.LogError("ObjectDisposedException sending '{message}'. Restarting connection...", text.Trim()); KillClient(SocketStatus.SOCKET_STATUS_CONNECT_FAILED); - ReconnectTimer.Reset(); + StopReconnectTimer(); } catch (Exception ex) { @@ -511,12 +534,12 @@ namespace PepperDash.Core { try { - if (Client != null && TheStream != null && IsConnected) + if (client != null && shellStream != null && IsConnected) { this.PrintSentBytes(bytes); - TheStream.Write(bytes, 0, bytes.Length); - TheStream.Flush(); + shellStream.Write(bytes, 0, bytes.Length); + shellStream.Flush(); } else { @@ -528,7 +551,7 @@ namespace PepperDash.Core this.LogException(ex, "ObjectDisposedException sending {message}", ComTextHelper.GetEscapedText(bytes)); KillClient(SocketStatus.SOCKET_STATUS_CONNECT_FAILED); - ReconnectTimer.Reset(); + StopReconnectTimer(); } catch (Exception ex) { @@ -537,6 +560,83 @@ namespace PepperDash.Core } #endregion + /// + /// Safely starts the reconnect timer with exception handling + /// + private void StartReconnectTimer() + { + try + { + reconnectTimer?.Change(AutoReconnectIntervalMs, System.Threading.Timeout.Infinite); + } + catch (ObjectDisposedException) + { + // Timer was disposed, ignore + this.LogDebug("Attempted to start timer but it was already disposed"); + } + } + + /// + /// Safely stops the reconnect timer with exception handling + /// + private void StopReconnectTimer() + { + try + { + reconnectTimer?.Change(System.Threading.Timeout.Infinite, System.Threading.Timeout.Infinite); + } + catch (ObjectDisposedException) + { + // Timer was disposed, ignore + this.LogDebug("Attempted to stop timer but it was already disposed"); + } + } + + /// + /// Deactivate method - properly dispose of resources + /// + public override bool Deactivate() + { + try + { + this.LogDebug("Deactivating SSH client - disposing resources"); + + // Stop trying reconnects + ConnectEnabled = false; + StopReconnectTimer(); + + // Disconnect and cleanup client + KillClient(SocketStatus.SOCKET_STATUS_BROKEN_LOCALLY); + + // Dispose timer + try + { + reconnectTimer?.Dispose(); + } + catch (ObjectDisposedException) + { + // Already disposed, ignore + } + + // Dispose semaphore + try + { + connectLock?.Dispose(); + } + catch (ObjectDisposedException) + { + // Already disposed, ignore + } + + return base.Deactivate(); + } + catch (Exception ex) + { + this.LogException(ex, "Error during SSH client deactivation"); + return false; + } + } + } //***************************************************************************************************** diff --git a/src/PepperDash.Core/Comm/GenericTcpIpClient.cs b/src/PepperDash.Core/Comm/GenericTcpIpClient.cs index 17507851..96d0387a 100644 --- a/src/PepperDash.Core/Comm/GenericTcpIpClient.cs +++ b/src/PepperDash.Core/Comm/GenericTcpIpClient.cs @@ -548,6 +548,12 @@ namespace PepperDash.Core /// public int AutoReconnectIntervalMs { get; set; } + /// + /// When true, turns off echo for the SSH session + /// + [JsonProperty("disableSshEcho")] + public bool DisableSshEcho { get; set; } + /// /// Default constructor /// @@ -558,8 +564,7 @@ namespace PepperDash.Core AutoReconnectIntervalMs = 5000; Username = ""; Password = ""; + DisableSshEcho = false; } - } - } diff --git a/src/PepperDash.Core/Logging/Debug.cs b/src/PepperDash.Core/Logging/Debug.cs index 62a95f2c..877d8997 100644 --- a/src/PepperDash.Core/Logging/Debug.cs +++ b/src/PepperDash.Core/Logging/Debug.cs @@ -168,7 +168,7 @@ namespace PepperDash.Core .WriteTo.File(new RenderedCompactJsonFormatter(), logFilePath, rollingInterval: RollingInterval.Day, restrictedToMinimumLevel: LogEventLevel.Debug, - retainedFileCountLimit: CrestronEnvironment.DevicePlatform == eDevicePlatform.Appliance ? 30 : 60, + retainedFileCountLimit: CrestronEnvironment.DevicePlatform == eDevicePlatform.Appliance ? 7 : 14, levelSwitch: _fileLogLevelSwitch ); @@ -1081,9 +1081,6 @@ namespace PepperDash.Core /// Logs to Console when at-level, and all messages to error log /// [Obsolete("Use LogMessage methods, Will be removed in 2.2.0 and later versions")] - /// - /// Console method - /// public static void Console(uint level, ErrorLogLevel errorLogLevel, string format, params object[] items) { @@ -1096,9 +1093,6 @@ namespace PepperDash.Core /// it will only be written to the log. /// [Obsolete("Use LogMessage methods, Will be removed in 2.2.0 and later versions")] - /// - /// ConsoleWithLog method - /// public static void ConsoleWithLog(uint level, string format, params object[] items) { LogMessage(level, format, items); diff --git a/src/PepperDash.Essentials.Core/Comm and IR/CommFactory.cs b/src/PepperDash.Essentials.Core/Comm and IR/CommFactory.cs index a0723c4b..fbc46579 100644 --- a/src/PepperDash.Essentials.Core/Comm and IR/CommFactory.cs +++ b/src/PepperDash.Essentials.Core/Comm and IR/CommFactory.cs @@ -64,8 +64,11 @@ namespace PepperDash.Essentials.Core break; case eControlMethod.Ssh: { - var ssh = new GenericSshClient(deviceConfig.Key + "-ssh", c.Address, c.Port, c.Username, c.Password); - ssh.AutoReconnect = c.AutoReconnect; + var ssh = new GenericSshClient(deviceConfig.Key + "-ssh", c.Address, c.Port, c.Username, c.Password) + { + AutoReconnect = c.AutoReconnect, + DisableEcho = c.DisableSshEcho + }; if (ssh.AutoReconnect) ssh.AutoReconnectIntervalMs = c.AutoReconnectIntervalMs; comm = ssh; @@ -73,8 +76,10 @@ namespace PepperDash.Essentials.Core } case eControlMethod.Tcpip: { - var tcp = new GenericTcpIpClient(deviceConfig.Key + "-tcp", c.Address, c.Port, c.BufferSize); - tcp.AutoReconnect = c.AutoReconnect; + var tcp = new GenericTcpIpClient(deviceConfig.Key + "-tcp", c.Address, c.Port, c.BufferSize) + { + AutoReconnect = c.AutoReconnect + }; if (tcp.AutoReconnect) tcp.AutoReconnectIntervalMs = c.AutoReconnectIntervalMs; comm = tcp; @@ -90,8 +95,10 @@ namespace PepperDash.Essentials.Core break; case eControlMethod.SecureTcpIp: { - var secureTcp = new GenericSecureTcpIpClient(deviceConfig.Key + "-secureTcp", c.Address, c.Port, c.BufferSize); - secureTcp.AutoReconnect = c.AutoReconnect; + var secureTcp = new GenericSecureTcpIpClient(deviceConfig.Key + "-secureTcp", c.Address, c.Port, c.BufferSize) + { + AutoReconnect = c.AutoReconnect + }; if (secureTcp.AutoReconnect) secureTcp.AutoReconnectIntervalMs = c.AutoReconnectIntervalMs; comm = secureTcp;