Merge pull request #2 in PEC/pepperdash-simplsharp-core from bugfix/fix-ssh-20161205 to master

* commit '422947559eee5739fea933cb3dcb7b4097bcfd73': Removed handler for stream error Ssh Connect() now spins up new clients. Seems to be helping deal with connection failures Trying and failing lots of things to fix SshClient. About to try making and destroying the entire client Hunting down program crash when network cable is pulled from server. Client.ErrorOccurred is not catching this Working through elusive disconnect bugs on SSH, with Amazon on CP3N
2026-02-11 10:44:49 +00:00 · 2016-12-07 10:02:38 -05:00
parent ad0140b6a3 422947559e
commit d90381c695
12 changed files with 151 additions and 92 deletions
--- a/Core/Comm/GenericSocketStatusChangeEventArgs.cs
+++ b/Core/Comm/GenericSocketStatusChangeEventArgs.cs
@@ -9,4 +9,21 @@ using Crestron.SimplSharp.CrestronSockets;
 namespace PepperDash.Core
 {
 	public delegate void GenericSocketStatusChangeEventDelegate(ISocketStatus client);
+
+
+	/// <summary>
+	/// 
+	/// </summary>
+	public class GenericSocketStatusChageEventArgs : EventArgs
+	{
+		public ISocketStatus Client { get; private set; }
+
+		public GenericSocketStatusChageEventArgs() { }
+
+		public GenericSocketStatusChageEventArgs(ISocketStatus client)
+		{
+			Client = client;
+		}
+	}
+
 }
--- a/Core/Comm/GenericSshClient.cs
+++ b/Core/Comm/GenericSshClient.cs
@@ -26,13 +26,12 @@ namespace PepperDash.Core
 		/// <summary>
 		/// Event when the connection status changes.
 		/// </summary>
-		//[Obsolete("Use SocketStatusChange instead")]
-		//public event EventHandler<SshConnectionChangeEventArgs> ConnectionChange;
+		public event EventHandler<GenericSocketStatusChageEventArgs> ConnectionChange;

 		/// <summary>
 		/// 
 		/// </summary>
-		public event GenericSocketStatusChangeEventDelegate SocketStatusChange;
+		//public event GenericSocketStatusChangeEventDelegate SocketStatusChange;

 		/// <summary>
 		/// Address of server
@@ -63,6 +62,14 @@ namespace PepperDash.Core
 			get { return ClientStatus == SocketStatus.SOCKET_STATUS_CONNECTED; }
 		}

+		/// <summary>
+		/// S+ helper for IsConnected
+		/// </summary>
+		public ushort UIsConnected
+		{
+			get { return (ushort)(IsConnected ? 1 : 0); }
+		}
+
 		/// <summary>
 		/// 
 		/// </summary>
@@ -93,6 +100,11 @@ namespace PepperDash.Core
 		/// </summary>
 		public bool AutoReconnect { get; set; }

+		/// <summary>
+		/// Will be set and unset by connect and disconnect only
+		/// </summary>
+		public bool ConnectEnabled { get; private set; }
+
 		/// <summary>
 		/// S+ helper for AutoReconnect
 		/// </summary>
@@ -109,13 +121,15 @@ namespace PepperDash.Core
 		public int AutoReconnectIntervalMs { get; set; }

 		SshClient Client;
+
 		ShellStream TheStream;
+
 		CTimer ReconnectTimer;

-		string PreviousHostname;
-		int PreviousPort;
-		string PreviousUsername;
-		string PreviousPassword;
+		//string PreviousHostname;
+		//int PreviousPort;
+		//string PreviousUsername;
+		//string PreviousPassword;

 		/// <summary>
 		/// Typical constructor.
@@ -171,7 +185,8 @@ namespace PepperDash.Core
 		/// </summary>
 		public void Connect()
 		{
-			Debug.Console(1, this, "attempting connect, IsConnected={0}", Client != null ? Client.IsConnected : false);
+			ConnectEnabled = true;
+			Debug.Console(1, this, "attempting connect");
 			
 			// Cancel reconnect if running.
 			if (ReconnectTimer != null)
@@ -197,55 +212,56 @@ namespace PepperDash.Core
 			kauth.AuthenticationPrompt += new EventHandler<AuthenticationPromptEventArgs>(kauth_AuthenticationPrompt);
 			PasswordAuthenticationMethod pauth = new PasswordAuthenticationMethod(Username, Password);

-			// always spin up new client in case parameters have changed
-			// **** MAY WANT TO CHANGE THIS BECAUSE OF SOCKET LEAKS ****
-			//if (Client != null)
-			//{
-			//    Client.Disconnect();
-			//    Client = null;
-			//}
-
 			// Make a new client if we need it or things have changed
-			if (Client == null || PropertiesHaveChanged())
-			{
+			//if (Client == null || PropertiesHaveChanged())
+			//{
 				if (Client != null)
 				{
 					Debug.Console(2, this, "Cleaning up disconnected client");
 					Client.ErrorOccurred -= Client_ErrorOccurred;
-					if(TheStream != null)
-						TheStream.DataReceived -= Stream_DataReceived;
-					TheStream = null;
+					KillStream();
+					
+					//if (TheStream != null)
+					//{
+					//    TheStream.DataReceived -= Stream_DataReceived;
+					//    TheStream.ErrorOccurred -= TheStream_ErrorOccurred;
+					//}
+					//TheStream = null;
 				}

 				Debug.Console(2, this, "Creating new SshClient");
 				ConnectionInfo connectionInfo = new ConnectionInfo(Hostname, Port, Username, pauth, kauth);
 				Client = new SshClient(connectionInfo);
 				Client.ErrorOccurred += Client_ErrorOccurred;
-			} 
-			PreviousHostname = Hostname;
-			PreviousPassword = Password;
-			PreviousPort = Port;
-			PreviousUsername = Username;
+			//} 
+			//PreviousHostname = Hostname;
+			//PreviousPassword = Password;
+			//PreviousPort = Port;
+			//PreviousUsername = Username;

 			//You can do it!
 			ClientStatus = SocketStatus.SOCKET_STATUS_WAITING;
 			try
 			{
 				Client.Connect();
-				if (Client.IsConnected)
-				{
-					Client.KeepAliveInterval = TimeSpan.FromSeconds(2);
-					Client.SendKeepAlive();
+
+				// Have to assume client is connected cause Client.IsConnected is busted in some cases
+				// All other conditions *should* error out...
+				//if (Client.IsConnected)
+				//{
+					//Client.KeepAliveInterval = TimeSpan.FromSeconds(2);
+					//Client.SendKeepAlive();
 					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;
-					PreviousHostname = Hostname;
-					PreviousPassword = Password;
-					PreviousPort = Port;
-					PreviousUsername = Username;
-				}
-				return;
+					//PreviousHostname = Hostname;
+					//PreviousPassword = Password;
+					//PreviousPort = Port;
+					//PreviousUsername = Username;
+				//}
+				return; // Success will not pass here
 			}
 			catch (SshConnectionException e)
 			{
@@ -269,54 +285,44 @@ namespace PepperDash.Core
 			}
 			
 			// Sucess will not make it this far
-			Client.Disconnect();
 			ClientStatus = SocketStatus.SOCKET_STATUS_CONNECT_FAILED;
 			HandleConnectionFailure();
 		}

+
+
 		/// <summary>
 		/// Disconnect the clients and put away it's resources.
 		/// </summary>
 		public void Disconnect()
 		{
+			ConnectEnabled = false;
 			// Stop trying reconnects, if we are
 			if (ReconnectTimer != null)
 			{
 				ReconnectTimer.Stop();
 				ReconnectTimer = null;
 			}
-			if(TheStream != null)
-				TheStream.DataReceived -= Stream_DataReceived;
+			KillStream();
 			Client.Disconnect();
+			Client = null;
 			ClientStatus = SocketStatus.SOCKET_STATUS_BROKEN_LOCALLY;
 			Debug.Console(1, this, "Disconnected");
 		}

-		/// <summary>
-		/// 
-		/// </summary>
-		//void DiscoAndCleanup()
-		//{
-		//    if (Client != null)
-		//    {
-		//        Client.ErrorOccurred -= Client_ErrorOccurred;
-		//        TheStream.DataReceived -= Stream_DataReceived;
-		//        Debug.Console(2, this, "Cleaning up disconnected client");
-		//        Client.Disconnect();
-		//        Client.Dispose();
-		//        Client = null;
-		//    }
-		//}
-
 		/// <summary>
 		/// Anything to do with reestablishing connection on failures
 		/// </summary>
 		void HandleConnectionFailure()
 		{
-			Debug.Console(2, this, "Checking autoreconnect: {0}, {1}ms", 
-				AutoReconnect, AutoReconnectIntervalMs);
-			if (AutoReconnect)
+			if (Client != null)
+				Client.Disconnect();
+			KillStream();
+
+			if (AutoReconnect && ConnectEnabled)
 			{
+				Debug.Console(2, this, "Checking autoreconnect: {0}, {1}ms", 
+					AutoReconnect, AutoReconnectIntervalMs);
 				if (ReconnectTimer == null)// || !ReconnectTimerRunning)
 				{
 					ReconnectTimer = new CTimer(o =>
@@ -335,12 +341,24 @@ namespace PepperDash.Core
 			}
 		}

-		bool PropertiesHaveChanged()
+		void KillStream()
 		{
-			return Hostname != PreviousHostname || Port != PreviousPort
-				|| Username != PreviousUsername || Password != PreviousPassword;
+			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;
+		//}
+
 		/// <summary>
 		/// Handles the keyboard interactive authentication, should it be required.
 		/// </summary>
@@ -371,23 +389,29 @@ namespace PepperDash.Core
 			}
 		}

+		///// <summary>
+		///// Handler for errors on the stream...
+		///// </summary>
+		///// <param name="sender"></param>
+		///// <param name="e"></param>
+		//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();
+		//}
+
 		/// <summary>
 		/// Error event handler for client events - disconnect, etc.  Will forward those events via ConnectionChange
 		/// event
 		/// </summary>
 		void Client_ErrorOccurred(object sender, Crestron.SimplSharp.Ssh.Common.ExceptionEventArgs e)
 		{
-			Debug.Console(1, this, "SSH client error: {0}", e.Exception);
-			if (!(e.Exception is SshConnectionException))
-			{
+			if (e.Exception is SshConnectionException || e.Exception is System.Net.Sockets.SocketException)
 				Debug.Console(1, this, "Disconnected by remote");
-			}
-			if (Client != null)
-			{
-				Client.Disconnect();
-				//Client.Dispose();
-				//Client = null;
-			}
+			else
+				Debug.Console(1, this, "Unhandled SSH client error: {0}", e.Exception);
+
 			ClientStatus = SocketStatus.SOCKET_STATUS_BROKEN_REMOTELY;
 			HandleConnectionFailure();
 		}
@@ -397,12 +421,8 @@ namespace PepperDash.Core
 		/// </summary>
 		void OnConnectionChange()
 		{
-			//if(ConnectionChange != null)
-			//    ConnectionChange(this, new SshConnectionChangeEventArgs(IsConnected, this));
-
-			var handler = SocketStatusChange;
-			if (handler != null)
-				SocketStatusChange(this);
+			if (ConnectionChange != null)
+				ConnectionChange(this, new GenericSocketStatusChageEventArgs(this));
 		}

 		#region IBasicCommunication Members
--- a/Core/Comm/GenericTcpIpClient.cs
+++ b/Core/Comm/GenericTcpIpClient.cs
@@ -26,7 +26,8 @@ namespace PepperDash.Core
 		/// <summary>
 		/// 
 		/// </summary>
-		public event GenericSocketStatusChangeEventDelegate SocketStatusChange;
+		//public event GenericSocketStatusChangeEventDelegate SocketStatusChange;
+		public event EventHandler<GenericSocketStatusChageEventArgs> ConnectionChange;

 		/// <summary>
 		/// 
@@ -207,10 +208,14 @@ namespace PepperDash.Core
 					break;
 			}

-			// Relay the event
-			var handler = SocketStatusChange;
+			var handler = ConnectionChange;
 			if (handler != null)
-				SocketStatusChange(this);
+				ConnectionChange(this, new GenericSocketStatusChageEventArgs(this));
+
+			// Relay the event
+			//var handler = SocketStatusChange;
+			//if (handler != null)
+			//    SocketStatusChange(this);
 		}
 	}

--- a/Core/CommunicationExtras.cs
+++ b/Core/CommunicationExtras.cs
@@ -31,7 +31,7 @@ namespace PepperDash.Core
 	/// </summary>
 	public interface ISocketStatus : IBasicCommunication
 	{
-		event GenericSocketStatusChangeEventDelegate SocketStatusChange;
+		event EventHandler<GenericSocketStatusChageEventArgs> ConnectionChange;
 		SocketStatus ClientStatus { get; }
 	}

--- a/Core/Debug/Debug.cs
+++ b/Core/Debug/Debug.cs
@@ -32,11 +32,7 @@ namespace PepperDash.Core

 		static CTimer SaveTimer;

-		/// <summary>
-		/// This should called from the ControlSystem Initiailize method. Will attempt to call
-		/// CrestronDataStoreStatic.InitCrestronDataStore which may have been called elsewhere.
-		/// </summary>
-		public static void Initialize()
+		static Debug()
 		{
 			CrestronDataStoreStatic.InitCrestronDataStore();
 			if (CrestronEnvironment.RuntimeEnvironment == eRuntimeEnvironment.SimplSharpPro)
@@ -53,6 +49,27 @@ namespace PepperDash.Core
 			Level = Contexts.GetOrCreateItem("DEFAULT").Level;
 		}

+		/// <summary>
+		/// This should called from the ControlSystem Initiailize method. Will attempt to call
+		/// CrestronDataStoreStatic.InitCrestronDataStore which may have been called elsewhere.
+		/// </summary>
+		public static void Initialize()
+		{
+			//CrestronDataStoreStatic.InitCrestronDataStore();
+			//if (CrestronEnvironment.RuntimeEnvironment == eRuntimeEnvironment.SimplSharpPro)
+			//{
+			//    // Add command to console
+			//    CrestronConsole.AddNewConsoleCommand(SetDebugFromConsole, "appdebug",
+			//        "appdebug:P [0-2]: Sets the application's console debug message level",
+			//        ConsoleAccessLevelEnum.AccessOperator);
+			//}
+
+			//CrestronEnvironment.ProgramStatusEventHandler += CrestronEnvironment_ProgramStatusEventHandler;
+
+			//LoadMemory();
+			//Level = Contexts.GetOrCreateItem("DEFAULT").Level;
+		}
+
 		/// <summary>
 		/// Used to save memory when shutting down
 		/// </summary>
--- a/Core/PepperDash_Core.projectinfo
+++ b/Core/PepperDash_Core.projectinfo
--- a/Core/Properties/ControlSystem.cfg
+++ b/Core/Properties/ControlSystem.cfg
@@ -2,6 +2,6 @@
 <ControlSystem>
  <Name>MC3 SSH</Name>
  <Address>ssh 10.0.0.15</Address>
-  <ProgramSlot />
-  <Storage />
+  <ProgramSlot>Program01</ProgramSlot>
+  <Storage>Internal Flash</Storage>
 </ControlSystem>
--- a/Core/bin/PepperDash_Core.clz
+++ b/Core/bin/PepperDash_Core.clz
--- a/Core/bin/PepperDash_Core.config
+++ b/Core/bin/PepperDash_Core.config
@@ -10,8 +10,8 @@
    <ArchiveName />
  </RequiredInfo>
  <OptionalInfo>
-    <CompiledOn>11/28/2016 1:59:52 PM</CompiledOn>
-    <CompilerRev>1.0.0.25195</CompilerRev>
+    <CompiledOn>12/6/2016 4:09:28 PM</CompiledOn>
+    <CompilerRev>1.0.0.29083</CompilerRev>
  </OptionalInfo>
  <Plugin>
    <Version>Crestron.SIMPLSharp, Version=2.0.48.0, Culture=neutral, PublicKeyToken=812d080f93e2de10</Version>
--- a/Core/bin/PepperDash_Core.dll
+++ b/Core/bin/PepperDash_Core.dll
--- a/Core/bin/manifest.info
+++ b/Core/bin/manifest.info
@@ -1,4 +1,4 @@
-MainAssembly=PepperDash_Core.dll:91882e81937f6bb6c3592368dedeea09
+MainAssembly=PepperDash_Core.dll:987a92dfd5c1b46ff22b2570acef6436
 MainAssemblyMinFirmwareVersion=1.007.0017
 MainAssemblyResource=SimplSharpData.dat:315526abf906cded47fb0c7510266a7e
 ü
--- a/Core/bin/manifest.ser
+++ b/Core/bin/manifest.ser