Adds lock to ssh and tcp classes to prevent possible threading issues.

This commit is contained in:
Alex Johnson
2022-09-02 14:50:15 -04:00
parent b12e798041
commit c722542b04
3 changed files with 204 additions and 201 deletions

View File

@@ -14,6 +14,10 @@ namespace PepperDash.Core
public class GenericSshClient : Device, ISocketStatusWithStreamDebugging, IAutoReconnect public class GenericSshClient : Device, ISocketStatusWithStreamDebugging, IAutoReconnect
{ {
private const string SPlusKey = "Uninitialized SshClient"; private const string SPlusKey = "Uninitialized SshClient";
/// <summary>
/// Enables debugging to console
/// </summary>
public CommunicationStreamDebugging StreamDebugging { get; private set; } public CommunicationStreamDebugging StreamDebugging { get; private set; }
/// <summary> /// <summary>
@@ -31,11 +35,6 @@ namespace PepperDash.Core
/// </summary> /// </summary>
public event EventHandler<GenericSocketStatusChageEventArgs> ConnectionChange; public event EventHandler<GenericSocketStatusChageEventArgs> ConnectionChange;
/// <summary>
///
/// </summary>
//public event GenericSocketStatusChangeEventDelegate SocketStatusChange;
/// <summary> /// <summary>
/// Address of server /// Address of server
/// </summary> /// </summary>
@@ -62,12 +61,9 @@ namespace PepperDash.Core
public bool IsConnected public bool IsConnected
{ {
// returns false if no client or not connected // returns false if no client or not connected
get { return ClientStatus == SocketStatus.SOCKET_STATUS_CONNECTED; } get { return Client != null && ClientStatus == SocketStatus.SOCKET_STATUS_CONNECTED; }
} }
private bool IsConnecting = false;
private bool DisconnectLogged = false;
/// <summary> /// <summary>
/// S+ helper for IsConnected /// S+ helper for IsConnected
/// </summary> /// </summary>
@@ -132,10 +128,10 @@ namespace PepperDash.Core
CTimer ReconnectTimer; CTimer ReconnectTimer;
//string PreviousHostname; //Lock object to prevent simulatneous connect/disconnect operations
//int PreviousPort; private readonly object connectLock = new object();
//string PreviousUsername;
//string PreviousPassword; private bool DisconnectLogged = false;
/// <summary> /// <summary>
/// Typical constructor. /// Typical constructor.
@@ -153,8 +149,11 @@ namespace PepperDash.Core
AutoReconnectIntervalMs = 5000; AutoReconnectIntervalMs = 5000;
ReconnectTimer = new CTimer(o => ReconnectTimer = new CTimer(o =>
{
if (ConnectEnabled)
{ {
Connect(); Connect();
}
}, Timeout.Infinite); }, Timeout.Infinite);
} }
@@ -169,8 +168,11 @@ namespace PepperDash.Core
AutoReconnectIntervalMs = 5000; AutoReconnectIntervalMs = 5000;
ReconnectTimer = new CTimer(o => ReconnectTimer = new CTimer(o =>
{
if (ConnectEnabled)
{ {
Connect(); Connect();
}
}, Timeout.Infinite); }, Timeout.Infinite);
} }
@@ -202,36 +204,32 @@ namespace PepperDash.Core
/// </summary> /// </summary>
public void Connect() public void Connect()
{ {
if (IsConnecting)
{
Debug.Console(0, this, Debug.ErrorLogLevel.Warning, "Connection attempt in progress. Exiting Connect()");
return;
}
IsConnecting = true;
ConnectEnabled = true;
Debug.Console(1, this, "attempting connect");
// Cancel reconnect if running.
ReconnectTimer.Stop();
// Don't try to connect if already
if (IsConnected)
return;
// Don't go unless everything is here // Don't go unless everything is here
if (string.IsNullOrEmpty(Hostname) || Port < 1 || Port > 65535 if (string.IsNullOrEmpty(Hostname) || Port < 1 || Port > 65535
|| Username == null || Password == null) || Username == null || Password == null)
{ {
Debug.Console(1, this, Debug.ErrorLogLevel.Error, "Connect failed. Check hostname, port, username and password are set or not null"); Debug.Console(0, this, Debug.ErrorLogLevel.Error, "Connect failed. Check hostname, port, username and password are set or not null");
return; return;
} }
ConnectEnabled = true;
lock (connectLock)
{
if (IsConnected)
{
Debug.Console(0, this, Debug.ErrorLogLevel.Warning, "Connection already connected. Exiting Connect()");
}
else
{
Debug.Console(1, this, "Attempting connect");
// Cancel reconnect if running.
ReconnectTimer.Stop();
// Cleanup the old client if it already exists // Cleanup the old client if it already exists
if (Client != null) if (Client != null)
{ {
Debug.Console(1, this, "Cleaning up disconnected client"); Debug.Console(1, this, "Cleaning up disconnected client");
Client.ErrorOccurred -= Client_ErrorOccurred;
KillClient(SocketStatus.SOCKET_STATUS_BROKEN_LOCALLY); KillClient(SocketStatus.SOCKET_STATUS_BROKEN_LOCALLY);
} }
@@ -254,12 +252,9 @@ namespace PepperDash.Core
Client.Connect(); Client.Connect();
TheStream = Client.CreateShellStream("PDTShell", 100, 80, 100, 200, 65534); TheStream = Client.CreateShellStream("PDTShell", 100, 80, 100, 200, 65534);
TheStream.DataReceived += Stream_DataReceived; TheStream.DataReceived += Stream_DataReceived;
//TheStream.ErrorOccurred += TheStream_ErrorOccurred;
Debug.Console(1, this, Debug.ErrorLogLevel.Notice, "Connected"); Debug.Console(1, this, Debug.ErrorLogLevel.Notice, "Connected");
ClientStatus = SocketStatus.SOCKET_STATUS_CONNECTED; ClientStatus = SocketStatus.SOCKET_STATUS_CONNECTED;
IsConnecting = false;
DisconnectLogged = false; DisconnectLogged = false;
return; // Success will not pass here
} }
catch (SshConnectionException e) catch (SshConnectionException e)
{ {
@@ -280,33 +275,41 @@ namespace PepperDash.Core
Debug.Console(1, this, errorLogLevel, "Error on connect:\r({0})", e); Debug.Console(1, this, errorLogLevel, "Error on connect:\r({0})", e);
DisconnectLogged = true; DisconnectLogged = true;
ClientStatus = SocketStatus.SOCKET_STATUS_CONNECT_FAILED; KillClient(SocketStatus.SOCKET_STATUS_CONNECT_FAILED);
HandleConnectionFailure(); if (AutoReconnect)
{
Debug.Console(1, this, "Checking autoreconnect: {0}, {1}ms", AutoReconnect, AutoReconnectIntervalMs);
ReconnectTimer.Reset(AutoReconnectIntervalMs);
}
} }
catch (Exception e) catch (Exception e)
{ {
Debug.Console(1, this, Debug.ErrorLogLevel.Error, "Unhandled exception on connect:\r({0})", e); var errorLogLevel = DisconnectLogged == true ? Debug.ErrorLogLevel.None : Debug.ErrorLogLevel.Error;
ClientStatus = SocketStatus.SOCKET_STATUS_CONNECT_FAILED; Debug.Console(1, this, errorLogLevel, "Unhandled exception on connect:\r({0})", e);
HandleConnectionFailure(); DisconnectLogged = true;
KillClient(SocketStatus.SOCKET_STATUS_CONNECT_FAILED);
if (AutoReconnect)
{
Debug.Console(1, this, "Checking autoreconnect: {0}, {1}ms", AutoReconnect, AutoReconnectIntervalMs);
ReconnectTimer.Reset(AutoReconnectIntervalMs);
}
}
}
} }
ClientStatus = SocketStatus.SOCKET_STATUS_CONNECT_FAILED;
HandleConnectionFailure();
} }
/// <summary> /// <summary>
/// Disconnect the clients and put away it's resources. /// Disconnect the clients and put away it's resources.
/// </summary> /// </summary>
public void Disconnect() public void Disconnect()
{ {
ConnectEnabled = false; lock(connectLock)
{
// Stop trying reconnects, if we are // Stop trying reconnects, if we are
ReconnectTimer.Stop(); ReconnectTimer.Stop();
KillClient(SocketStatus.SOCKET_STATUS_BROKEN_LOCALLY); KillClient(SocketStatus.SOCKET_STATUS_BROKEN_LOCALLY);
} }
}
/// <summary> /// <summary>
/// Kills the stream, cleans up the client and sets it to null /// Kills the stream, cleans up the client and sets it to null
@@ -317,28 +320,18 @@ namespace PepperDash.Core
if (Client != null) if (Client != null)
{ {
IsConnecting = false; try
{
Client.Disconnect(); Client.Disconnect();
Client.Dispose();
Client = null; Client = null;
ClientStatus = status; ClientStatus = status;
Debug.Console(1, this, "Disconnected"); Debug.Console(1, this, "Disconnected client");
} }
catch (Exception ex)
{
Debug.Console(0, this, "Exception killing client: {0}", ex.Message);
} }
/// <summary>
/// Anything to do with reestablishing connection on failures
/// </summary>
void HandleConnectionFailure()
{
KillClient(SocketStatus.SOCKET_STATUS_CONNECT_FAILED);
Debug.Console(1, this, "Client nulled due to connection failure. AutoReconnect: {0}, ConnectEnabled: {1}", AutoReconnect, ConnectEnabled);
if (AutoReconnect && ConnectEnabled)
{
Debug.Console(1, this, "Checking autoreconnect: {0}, {1}ms", AutoReconnect, AutoReconnectIntervalMs);
ReconnectTimer.Reset(AutoReconnectIntervalMs);
Debug.Console(1, this, "Attempting connection in {0} seconds",
(float) (AutoReconnectIntervalMs/1000));
} }
} }
@@ -353,6 +346,7 @@ namespace PepperDash.Core
TheStream.Close(); TheStream.Close();
TheStream.Dispose(); TheStream.Dispose();
TheStream = null; TheStream = null;
Debug.Console(1, this, "Disconnected stream");
} }
} }
@@ -402,14 +396,24 @@ namespace PepperDash.Core
/// event /// event
/// </summary> /// </summary>
void Client_ErrorOccurred(object sender, Crestron.SimplSharp.Ssh.Common.ExceptionEventArgs e) void Client_ErrorOccurred(object sender, Crestron.SimplSharp.Ssh.Common.ExceptionEventArgs e)
{
CrestronInvoke.BeginInvoke(o =>
{ {
if (e.Exception is SshConnectionException || e.Exception is System.Net.Sockets.SocketException) if (e.Exception is SshConnectionException || e.Exception is System.Net.Sockets.SocketException)
Debug.Console(1, this, Debug.ErrorLogLevel.Error, "Disconnected by remote"); Debug.Console(1, this, Debug.ErrorLogLevel.Error, "Disconnected by remote");
else else
Debug.Console(1, this, Debug.ErrorLogLevel.Error, "Unhandled SSH client error: {0}", e.Exception); Debug.Console(1, this, Debug.ErrorLogLevel.Error, "Unhandled SSH client error: {0}", e.Exception);
ClientStatus = SocketStatus.SOCKET_STATUS_BROKEN_REMOTELY; lock (connectLock)
HandleConnectionFailure(); {
KillClient(SocketStatus.SOCKET_STATUS_BROKEN_REMOTELY);
}
if (AutoReconnect && ConnectEnabled)
{
Debug.Console(1, this, "Checking autoreconnect: {0}, {1}ms", AutoReconnect, AutoReconnectIntervalMs);
ReconnectTimer.Reset(AutoReconnectIntervalMs);
}
});
} }
/// <summary> /// <summary>
@@ -451,8 +455,6 @@ namespace PepperDash.Core
Debug.Console(0, "Stack Trace: {0}", ex.StackTrace); Debug.Console(0, "Stack Trace: {0}", ex.StackTrace);
Debug.Console(1, this, Debug.ErrorLogLevel.Error, "Stream write failed. Disconnected, closing"); Debug.Console(1, this, Debug.ErrorLogLevel.Error, "Stream write failed. Disconnected, closing");
ClientStatus = SocketStatus.SOCKET_STATUS_BROKEN_REMOTELY;
HandleConnectionFailure();
} }
} }
@@ -480,8 +482,6 @@ namespace PepperDash.Core
catch catch
{ {
Debug.Console(1, this, Debug.ErrorLogLevel.Error, "Stream write failed. Disconnected, closing"); Debug.Console(1, this, Debug.ErrorLogLevel.Error, "Stream write failed. Disconnected, closing");
ClientStatus = SocketStatus.SOCKET_STATUS_BROKEN_REMOTELY;
HandleConnectionFailure();
} }
} }

View File

@@ -5,7 +5,6 @@ using System.Text;
using System.Text.RegularExpressions; using System.Text.RegularExpressions;
using Crestron.SimplSharp; using Crestron.SimplSharp;
using Crestron.SimplSharp.CrestronSockets; using Crestron.SimplSharp.CrestronSockets;
using Newtonsoft.Json; using Newtonsoft.Json;
using Newtonsoft.Json.Linq; using Newtonsoft.Json.Linq;
@@ -17,6 +16,10 @@ namespace PepperDash.Core
public class GenericTcpIpClient : Device, ISocketStatusWithStreamDebugging, IAutoReconnect public class GenericTcpIpClient : Device, ISocketStatusWithStreamDebugging, IAutoReconnect
{ {
private const string SplusKey = "Uninitialized TcpIpClient"; private const string SplusKey = "Uninitialized TcpIpClient";
/// <summary>
/// Enables debugging to console
/// </summary>
public CommunicationStreamDebugging StreamDebugging { get; private set; } public CommunicationStreamDebugging StreamDebugging { get; private set; }
/// <summary> /// <summary>
@@ -167,6 +170,9 @@ namespace PepperDash.Core
get { return Client.ClientStatus == SocketStatus.SOCKET_STATUS_CONNECTED; } get { return Client.ClientStatus == SocketStatus.SOCKET_STATUS_CONNECTED; }
} }
//Lock object to prevent simulatneous connect/disconnect operations
private readonly object connectLock = new object();
CTimer RetryTimer; CTimer RetryTimer;
/// <summary> /// <summary>
@@ -187,12 +193,7 @@ namespace PepperDash.Core
RetryTimer = new CTimer(o => RetryTimer = new CTimer(o =>
{ {
if (Client == null) Reconnect();
{
return;
}
Client.ConnectToServerAsync(ConnectToServerCallback);
}, Timeout.Infinite); }, Timeout.Infinite);
CrestronEnvironment.ProgramStatusEventHandler += new ProgramStatusEventHandler(CrestronEnvironment_ProgramStatusEventHandler); CrestronEnvironment.ProgramStatusEventHandler += new ProgramStatusEventHandler(CrestronEnvironment_ProgramStatusEventHandler);
@@ -212,12 +213,7 @@ namespace PepperDash.Core
RetryTimer = new CTimer(o => RetryTimer = new CTimer(o =>
{ {
if (Client == null) Reconnect();
{
return;
}
Client.ConnectToServerAsync(ConnectToServerCallback);
}, Timeout.Infinite); }, Timeout.Infinite);
} }
@@ -234,12 +230,7 @@ namespace PepperDash.Core
RetryTimer = new CTimer(o => RetryTimer = new CTimer(o =>
{ {
if (Client == null) Reconnect();
{
return;
}
Client.ConnectToServerAsync(ConnectToServerCallback);
}, Timeout.Infinite); }, Timeout.Infinite);
} }
@@ -259,7 +250,7 @@ namespace PepperDash.Core
if (programEventType == eProgramStatusEventType.Stopping) if (programEventType == eProgramStatusEventType.Stopping)
{ {
Debug.Console(1, this, "Program stopping. Closing connection"); Debug.Console(1, this, "Program stopping. Closing connection");
Disconnect(); Deactivate();
} }
} }
@@ -269,6 +260,8 @@ namespace PepperDash.Core
/// <returns></returns> /// <returns></returns>
public override bool Deactivate() public override bool Deactivate()
{ {
RetryTimer.Stop();
RetryTimer.Dispose();
if (Client != null) if (Client != null)
{ {
Client.SocketStatusChange -= this.Client_SocketStatusChange; Client.SocketStatusChange -= this.Client_SocketStatusChange;
@@ -282,9 +275,6 @@ namespace PepperDash.Core
/// </summary> /// </summary>
public void Connect() public void Connect()
{ {
if (IsConnected)
DisconnectClient();
if (string.IsNullOrEmpty(Hostname)) if (string.IsNullOrEmpty(Hostname))
{ {
Debug.Console(1, Debug.ErrorLogLevel.Warning, "GenericTcpIpClient '{0}': No address set", Key); Debug.Console(1, Debug.ErrorLogLevel.Warning, "GenericTcpIpClient '{0}': No address set", Key);
@@ -298,32 +288,58 @@ namespace PepperDash.Core
} }
} }
if (Client == null) lock(connectLock)
{ {
if (IsConnected)
{
Debug.Console(0, this, Debug.ErrorLogLevel.Warning, "Connection already connected. Exiting Connect()");
}
else
{
//Stop retry timer if running
RetryTimer.Stop();
Client = new TCPClient(Hostname, Port, BufferSize); Client = new TCPClient(Hostname, Port, BufferSize);
Client.SocketStatusChange -= Client_SocketStatusChange; Client.SocketStatusChange -= Client_SocketStatusChange;
Client.SocketStatusChange += Client_SocketStatusChange; Client.SocketStatusChange += Client_SocketStatusChange;
}
DisconnectCalledByUser = false; DisconnectCalledByUser = false;
Client.ConnectToServerAsync(ConnectToServerCallback); // (null); Client.ConnectToServerAsync(ConnectToServerCallback);
}
}
}
private void Reconnect()
{
if (Client == null)
{
return;
}
lock (connectLock)
{
if (IsConnected || DisconnectCalledByUser == true)
{
Debug.Console(0, this, Debug.ErrorLogLevel.Warning, "Reconnect no longer needed. Exiting Reconnect()");
}
else
{
Client.ConnectToServerAsync(ConnectToServerCallback);
}
}
} }
/// <summary> /// <summary>
/// Attempts to disconnect the client /// Attempts to disconnect the client
/// </summary> /// </summary>
public void Disconnect() public void Disconnect()
{
lock (connectLock)
{ {
DisconnectCalledByUser = true; DisconnectCalledByUser = true;
// Stop trying reconnects, if we are // Stop trying reconnects, if we are
RetryTimer.Stop(); RetryTimer.Stop();
if (Client != null)
{
DisconnectClient(); DisconnectClient();
Client = null;
Debug.Console(1, this, "Disconnected");
} }
} }
@@ -335,7 +351,7 @@ namespace PepperDash.Core
if (Client != null) if (Client != null)
{ {
Debug.Console(1, this, "Disconnecting client"); Debug.Console(1, this, "Disconnecting client");
if(IsConnected) if (IsConnected)
Client.DisconnectFromServer(); Client.DisconnectFromServer();
} }
} }
@@ -347,7 +363,7 @@ namespace PepperDash.Core
void ConnectToServerCallback(TCPClient c) void ConnectToServerCallback(TCPClient c)
{ {
Debug.Console(1, this, "Server connection result: {0}", c.ClientStatus); Debug.Console(1, this, "Server connection result: {0}", c.ClientStatus);
if (c.ClientStatus != SocketStatus.SOCKET_STATUS_CONNECTED && AutoReconnect) if (c.ClientStatus != SocketStatus.SOCKET_STATUS_CONNECTED)
WaitAndTryReconnect(); WaitAndTryReconnect();
} }
@@ -356,16 +372,18 @@ namespace PepperDash.Core
/// </summary> /// </summary>
void WaitAndTryReconnect() void WaitAndTryReconnect()
{ {
DisconnectClient(); CrestronInvoke.BeginInvoke(o =>
if (Client != null)
{ {
lock (connectLock)
{
if (!IsConnected && AutoReconnect && !DisconnectCalledByUser && Client != null)
{
DisconnectClient();
Debug.Console(1, this, "Attempting reconnect, status={0}", Client.ClientStatus); Debug.Console(1, this, "Attempting reconnect, status={0}", Client.ClientStatus);
if (!DisconnectCalledByUser)
RetryTimer.Reset(AutoReconnectIntervalMs); RetryTimer.Reset(AutoReconnectIntervalMs);
} }
}
});
} }
/// <summary> /// <summary>
@@ -398,7 +416,6 @@ namespace PepperDash.Core
Debug.Console(0, this, "Received {1} characters of text: '{0}'", ComTextHelper.GetDebugText(str), str.Length); Debug.Console(0, this, "Received {1} characters of text: '{0}'", ComTextHelper.GetDebugText(str), str.Length);
textHandler(this, new GenericCommMethodReceiveTextArgs(str)); textHandler(this, new GenericCommMethodReceiveTextArgs(str));
} }
} }
client.ReceiveDataAsync(Receive); client.ReceiveDataAsync(Receive);
@@ -416,8 +433,6 @@ namespace PepperDash.Core
Debug.Console(0, this, "Sending {0} characters of text: '{1}'", text.Length, ComTextHelper.GetDebugText(text)); Debug.Console(0, this, "Sending {0} characters of text: '{1}'", text.Length, ComTextHelper.GetDebugText(text));
if(Client != null) if(Client != null)
Client.SendData(bytes, bytes.Length); Client.SendData(bytes, bytes.Length);
} }
/// <summary> /// <summary>
@@ -453,26 +468,18 @@ namespace PepperDash.Core
void Client_SocketStatusChange(TCPClient client, SocketStatus clientSocketStatus) void Client_SocketStatusChange(TCPClient client, SocketStatus clientSocketStatus)
{ {
Debug.Console(1, this, "Socket status change {0} ({1})", clientSocketStatus, ClientStatusText); Debug.Console(1, this, "Socket status change {0} ({1})", clientSocketStatus, ClientStatusText);
if (client.ClientStatus != SocketStatus.SOCKET_STATUS_CONNECTED && !DisconnectCalledByUser && AutoReconnect) if (client.ClientStatus != SocketStatus.SOCKET_STATUS_CONNECTED)
WaitAndTryReconnect(); WaitAndTryReconnect();
// Probably doesn't need to be a switch since all other cases were eliminated if(clientSocketStatus == SocketStatus.SOCKET_STATUS_CONNECTED)
switch (clientSocketStatus)
{ {
case SocketStatus.SOCKET_STATUS_CONNECTED:
Client.ReceiveDataAsync(Receive); Client.ReceiveDataAsync(Receive);
DisconnectCalledByUser = false; DisconnectCalledByUser = false;
break;
} }
var handler = ConnectionChange; var handler = ConnectionChange;
if (handler != null) if (handler != null)
ConnectionChange(this, new GenericSocketStatusChageEventArgs(this)); ConnectionChange(this, new GenericSocketStatusChageEventArgs(this));
// Relay the event
//var handler = SocketStatusChange;
//if (handler != null)
// SocketStatusChange(this);
} }
} }

View File

@@ -537,10 +537,6 @@ namespace PepperDash.Core
//check for file at old path //check for file at old path
if (!File.Exists(oldFilePath)) if (!File.Exists(oldFilePath))
{ {
Console(0, ErrorLogLevel.Notice,
String.Format(
@"Debug settings file not found at \nvram\debugSettings\program{0}. Attempting to use file at \user\debugSettings\program{0}",
InitialParametersClass.ApplicationNumber));
return; return;
} }