Skip to content

Commit c274f1f

Browse files
committed
Catch and log SocketException that is thrown while shutting down the socket or clearing its read buffer.
Always log raised exception, even if we're disconnecting.
1 parent 879c39b commit c274f1f

File tree

1 file changed

+43
-34
lines changed

1 file changed

+43
-34
lines changed

src/Renci.SshNet/Session.cs

Lines changed: 43 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -290,6 +290,7 @@ public bool IsConnected
290290
public byte[] SessionId { get; private set; }
291291

292292
private Message _clientInitMessage;
293+
293294
/// <summary>
294295
/// Gets the client init message.
295296
/// </summary>
@@ -756,11 +757,11 @@ internal void WaitOnHandle(WaitHandle waitHandle, TimeSpan timeout)
756757
throw new ArgumentNullException("waitHandle");
757758

758759
var waitHandles = new[]
759-
{
760-
_exceptionWaitHandle,
761-
_messageListenerCompleted,
762-
waitHandle
763-
};
760+
{
761+
_exceptionWaitHandle,
762+
_messageListenerCompleted,
763+
waitHandle
764+
};
764765

765766
switch (WaitHandle.WaitAny(waitHandles, timeout))
766767
{
@@ -803,9 +804,7 @@ internal void SendMessage(Message message)
803804

804805
DiagnosticAbstraction.Log(string.Format("[{0}] SendMessage to server '{1}': '{2}'.", ToHex(SessionId), message.GetType().Name, message));
805806

806-
// Messages can be sent by different thread so we need to synchronize it
807-
var paddingMultiplier = _clientCipher == null ? (byte)8 : Math.Max((byte)8, _serverCipher.MinimumSize); // Should be recalculate base on cipher min length if cipher specified
808-
807+
var paddingMultiplier = _clientCipher == null ? (byte) 8 : Math.Max((byte) 8, _serverCipher.MinimumSize);
809808
var packetData = message.GetPacket(paddingMultiplier, _clientCompression);
810809

811810
// take a write lock to ensure the outbound packet sequence number is incremented
@@ -845,7 +844,7 @@ internal void SendMessage(Message message)
845844
}
846845
else
847846
{
848-
var data = new byte[packetLength + (_clientMac.HashSize / 8)];
847+
var data = new byte[packetLength + (_clientMac.HashSize/8)];
849848
Buffer.BlockCopy(packetData, packetDataOffset, data, 0, packetLength);
850849
Buffer.BlockCopy(hash, 0, data, packetLength, hash.Length);
851850

@@ -915,7 +914,7 @@ private Message ReceiveMessage()
915914
// Determine the size of the first block, which is 8 or cipher block size (whichever is larger) bytes
916915
var blockSize = _serverCipher == null ? (byte) 8 : Math.Max((byte) 8, _serverCipher.MinimumSize);
917916

918-
var serverMacLength = _serverMac != null ? _serverMac.HashSize / 8 : 0;
917+
var serverMacLength = _serverMac != null ? _serverMac.HashSize/8 : 0;
919918

920919
byte[] data;
921920
uint packetLength;
@@ -1053,7 +1052,7 @@ private void HandleMessage<T>(T message) where T : Message
10531052
OnMessageReceived(message);
10541053
}
10551054

1056-
#region Handle transport messages
1055+
#region Handle transport messages
10571056

10581057
/// <summary>
10591058
/// Invoked via reflection.
@@ -1122,9 +1121,9 @@ private void HandleMessage(NewKeysMessage message)
11221121
OnNewKeysReceived(message);
11231122
}
11241123

1125-
#endregion
1124+
#endregion
11261125

1127-
#region Handle User Authentication messages
1126+
#region Handle User Authentication messages
11281127

11291128
/// <summary>
11301129
/// Invoked via reflection.
@@ -1158,9 +1157,9 @@ private void HandleMessage(BannerMessage message)
11581157
OnUserAuthenticationBannerReceived(message);
11591158
}
11601159

1161-
#endregion
1160+
#endregion
11621161

1163-
#region Handle connection messages
1162+
#region Handle connection messages
11641163

11651164
/// <summary>
11661165
/// Invoked via reflection.
@@ -1274,9 +1273,9 @@ private void HandleMessage(ChannelFailureMessage message)
12741273
OnChannelFailureReceived(message);
12751274
}
12761275

1277-
#endregion
1276+
#endregion
12781277

1279-
#region Handle received message events
1278+
#region Handle received message events
12801279

12811280
/// <summary>
12821281
/// Called when <see cref="DisconnectMessage"/> received.
@@ -1660,7 +1659,7 @@ protected virtual void OnMessageReceived(Message message)
16601659
handlers(this, new MessageEventArgs<Message>(message));
16611660
}
16621661

1663-
#endregion
1662+
#endregion
16641663

16651664
private void KeyExchange_HostKeyReceived(object sender, HostKeyEventArgs e)
16661665
{
@@ -1739,7 +1738,7 @@ internal static string ToHex(byte[] bytes)
17391738
return ToHex(bytes, 0);
17401739
}
17411740

1742-
#endregion
1741+
#endregion
17431742

17441743
/// <summary>
17451744
/// Gets a value indicating whether the socket is connected.
@@ -1865,22 +1864,33 @@ private void SocketDisconnectAndDispose()
18651864
{
18661865
if (_socket.Connected)
18671866
{
1868-
// interrupt any pending reads
1869-
_socket.Shutdown(SocketShutdown.Send);
1867+
try
1868+
{
1869+
// interrupt any pending reads; should be done outside of socket read lock as we
1870+
// actually want shutdown the socket to make sure blocking reads are interrupted
1871+
_socket.Shutdown(SocketShutdown.Send);
18701872

18711873
#if FEATURE_SOCKET_POLL
1872-
// since we've shut down the socket, there should not be any reads in progress but
1873-
// we still take a read lock to ensure IsSocketConnected continues to provide
1874-
// correct results
1875-
//
1876-
// only necessary if IsSocketConnected actually uses Socket.Poll.
1877-
lock (_socketReadLock)
1878-
{
1874+
// since we've shut down the socket, there should not be any reads in progress but
1875+
// we still take a read lock to ensure IsSocketConnected continues to provide
1876+
// correct results
1877+
//
1878+
// see IsSocketConnected for details on the race condition we avoid with this lock
1879+
//
1880+
// this race condition only exists if IsSocketConnected actually uses Socket.Poll,
1881+
// and as such this read is also only required when Socket.Poll is available
1882+
lock (_socketReadLock)
1883+
{
18791884
#endif // FEATURE_SOCKET_POLL
1880-
SocketAbstraction.ClearReadBuffer(_socket);
1885+
SocketAbstraction.ClearReadBuffer(_socket);
18811886
#if FEATURE_SOCKET_POLL
1882-
}
1887+
}
18831888
#endif // FEATURE_SOCKET_POLL
1889+
}
1890+
catch (SocketException ex)
1891+
{
1892+
DiagnosticAbstraction.Log("Failure shutting down socket or clearing read buffer: " + ex);
1893+
}
18841894
}
18851895

18861896
_socket.Dispose();
@@ -2229,11 +2239,13 @@ private void ConnectHttp()
22292239
/// <summary>
22302240
/// Raises the <see cref="ErrorOccured"/> event.
22312241
/// </summary>
2232-
/// <param name="exp">The exp.</param>
2242+
/// <param name="exp">The <see cref="Exception"/>.</param>
22332243
private void RaiseError(Exception exp)
22342244
{
22352245
var connectionException = exp as SshConnectionException;
22362246

2247+
DiagnosticAbstraction.Log(string.Format("[{0}] {1} Raised exception: {2}", ToHex(SessionId), DateTime.Now.Ticks, exp));
2248+
22372249
if (_isDisconnecting)
22382250
{
22392251
// a connection exception which is raised while isDisconnecting is normal and
@@ -2248,10 +2260,7 @@ private void RaiseError(Exception exp)
22482260
return;
22492261
}
22502262

2251-
DiagnosticAbstraction.Log(string.Format("[{0}] {1} Raised exception: {2}", ToHex(SessionId), DateTime.Now.Ticks, exp));
2252-
22532263
_exception = exp;
2254-
22552264
_exceptionWaitHandle.Set();
22562265

22572266
var errorOccured = ErrorOccured;

0 commit comments

Comments
 (0)