Skip to content

Commit 69fb3be

Browse files
driesengdrieseng
committed
Signal the closed wait handle to avoid a deadlock when a subscriber to the Closed event in turn closes or disposes the channel.
1 parent a6f5d35 commit 69fb3be

File tree

3 files changed

+145
-5
lines changed

3 files changed

+145
-5
lines changed

‎src/Renci.SshNet.Tests/Classes/Channels/ChannelSessionTest_Dispose_SessionIsConnectedAndChannelIsOpen_ChannelCloseAndChannelEofReceived_DisposeInEventHandler.cs

Lines changed: 136 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,136 @@
1+
using System;
2+
using System.Collections.Generic;
3+
using System.Threading;
4+
using Microsoft.VisualStudio.TestTools.UnitTesting;
5+
using Moq;
6+
using Renci.SshNet.Channels;
7+
using Renci.SshNet.Common;
8+
using Renci.SshNet.Messages.Connection;
9+
using Renci.SshNet.Tests.Common;
10+
11+
namespace Renci.SshNet.Tests.Classes.Channels
12+
{
13+
[TestClass]
14+
public class ChannelSessionTest_Dispose_SessionIsConnectedAndChannelIsOpen_ChannelCloseAndChannelEofReceived_DisposeInEventHandler
15+
{
16+
private Mock<ISession> _sessionMock;
17+
private uint _localChannelNumber;
18+
private uint _localWindowSize;
19+
private uint _localPacketSize;
20+
private uint _remoteChannelNumber;
21+
private uint _remoteWindowSize;
22+
private uint _remotePacketSize;
23+
private IList<ChannelEventArgs> _channelClosedRegister;
24+
private List<ExceptionEventArgs> _channelExceptionRegister;
25+
private ChannelSession _channel;
26+
private Mock<IConnectionInfo> _connectionInfoMock;
27+
private MockSequence _sequence;
28+
private SemaphoreLight _sessionSemaphore;
29+
private int _initialSessionSemaphoreCount;
30+
31+
[TestInitialize]
32+
public void Initialize()
33+
{
34+
Arrange();
35+
Act();
36+
}
37+
38+
private void Arrange()
39+
{
40+
var random = new Random();
41+
_localChannelNumber = (uint)random.Next(0, int.MaxValue);
42+
_localWindowSize = (uint)random.Next(0, int.MaxValue);
43+
_localPacketSize = (uint)random.Next(0, int.MaxValue);
44+
_remoteChannelNumber = (uint)random.Next(0, int.MaxValue);
45+
_remoteWindowSize = (uint)random.Next(0, int.MaxValue);
46+
_remotePacketSize = (uint)random.Next(0, int.MaxValue);
47+
_channelClosedRegister = new List<ChannelEventArgs>();
48+
_channelExceptionRegister = new List<ExceptionEventArgs>();
49+
_initialSessionSemaphoreCount = random.Next(10, 20);
50+
_sessionSemaphore = new SemaphoreLight(_initialSessionSemaphoreCount);
51+
52+
_sessionMock = new Mock<ISession>(MockBehavior.Strict);
53+
_connectionInfoMock = new Mock<IConnectionInfo>(MockBehavior.Strict);
54+
55+
_sequence = new MockSequence();
56+
_sessionMock.InSequence(_sequence).Setup(p => p.ConnectionInfo).Returns(_connectionInfoMock.Object);
57+
_connectionInfoMock.InSequence(_sequence).Setup(p => p.RetryAttempts).Returns(1);
58+
_sessionMock.Setup(p => p.SessionSemaphore).Returns(_sessionSemaphore);
59+
_sessionMock.InSequence(_sequence)
60+
.Setup(
61+
p =>
62+
p.SendMessage(
63+
It.Is<ChannelOpenMessage>(
64+
m =>
65+
m.LocalChannelNumber == _localChannelNumber &&
66+
m.InitialWindowSize == _localWindowSize && m.MaximumPacketSize == _localPacketSize &&
67+
m.Info is SessionChannelOpenInfo)));
68+
_sessionMock.InSequence(_sequence)
69+
.Setup(p => p.WaitOnHandle(It.IsNotNull<WaitHandle>()))
70+
.Callback<WaitHandle>(
71+
w =>
72+
{
73+
_sessionMock.Raise(
74+
s => s.ChannelOpenConfirmationReceived += null,
75+
new MessageEventArgs<ChannelOpenConfirmationMessage>(
76+
new ChannelOpenConfirmationMessage(
77+
_localChannelNumber,
78+
_remoteWindowSize,
79+
_remotePacketSize,
80+
_remoteChannelNumber)));
81+
w.WaitOne();
82+
});
83+
_sessionMock.InSequence(_sequence).Setup(p => p.IsConnected).Returns(true);
84+
_sessionMock.InSequence(_sequence)
85+
.Setup(p => p.TrySendMessage(It.Is<ChannelCloseMessage>(c => c.LocalChannelNumber == _remoteChannelNumber)))
86+
.Returns(true);
87+
_sessionMock.InSequence(_sequence)
88+
.Setup(s => s.WaitOnHandle(It.IsNotNull<EventWaitHandle>()))
89+
.Callback<WaitHandle>(w => w.WaitOne());
90+
91+
_channel = new ChannelSession(_sessionMock.Object, _localChannelNumber, _localWindowSize, _localPacketSize);
92+
_channel.Closed += (sender, args) =>
93+
{
94+
_channelClosedRegister.Add(args);
95+
_channel.Dispose();
96+
};
97+
_channel.Exception += (sender, args) => _channelExceptionRegister.Add(args);
98+
_channel.Open();
99+
100+
_sessionMock.Raise(p => p.ChannelEofReceived += null,
101+
new MessageEventArgs<ChannelEofMessage>(new ChannelEofMessage(_localChannelNumber)));
102+
_sessionMock.Raise(p => p.ChannelCloseReceived += null,
103+
new MessageEventArgs<ChannelCloseMessage>(new ChannelCloseMessage(_localChannelNumber)));
104+
}
105+
106+
private void Act()
107+
{
108+
_channel.Dispose();
109+
}
110+
111+
[TestMethod]
112+
public void CurrentCountOfSessionSemaphoreShouldBeEqualToInitialCount()
113+
{
114+
Assert.AreEqual(_initialSessionSemaphoreCount, _sessionSemaphore.CurrentCount);
115+
}
116+
117+
[TestMethod]
118+
public void ExceptionShouldNeverHaveFired()
119+
{
120+
Assert.AreEqual(0, _channelExceptionRegister.Count, _channelExceptionRegister.AsString());
121+
}
122+
123+
[TestMethod]
124+
public void ClosedEventShouldHaveFiredOnce()
125+
{
126+
Assert.AreEqual(1, _channelClosedRegister.Count);
127+
Assert.AreEqual(_localChannelNumber, _channelClosedRegister[0].ChannelNumber);
128+
}
129+
130+
[TestMethod]
131+
public void IsOpenShouldReturnFalse()
132+
{
133+
Assert.IsFalse(_channel.IsOpen);
134+
}
135+
}
136+
}

‎src/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,7 @@
9494
<Compile Include="Classes\Channels\ChannelSessionTest_Dispose_Disposed.cs" />
9595
<Compile Include="Classes\Channels\ChannelSessionTest_Dispose_SessionIsConnectedAndChannelIsOpen_ChannelCloseAndChannelEofReceived_SendChannelCloseMessageFailure.cs" />
9696
<Compile Include="Classes\Channels\ChannelSessionTest_Dispose_SessionIsConnectedAndChannelIsOpen_ChannelCloseAndChannelEofReceived_SendChannelCloseMessageSuccess.cs" />
97+
<Compile Include="Classes\Channels\ChannelSessionTest_Dispose_SessionIsConnectedAndChannelIsOpen_ChannelCloseAndChannelEofReceived_DisposeInEventHandler.cs" />
9798
<Compile Include="Classes\Channels\ChannelSessionTest_Dispose_SessionIsConnectedAndChannelIsOpen_ChannelCloseReceived_SendChannelCloseMessageFailure.cs" />
9899
<Compile Include="Classes\Channels\ChannelSessionTest_Dispose_SessionIsConnectedAndChannelIsOpen_ChannelCloseReceived_SendChannelCloseMessageSuccess.cs" />
99100
<Compile Include="Classes\Channels\ChannelSessionTest_Dispose_SessionIsConnectedAndChannelIsOpen_ChannelEofReceived_SendChannelCloseMessageFailure.cs" />

‎src/Renci.SshNet/Channels/Channel.cs

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -400,18 +400,21 @@ protected virtual void OnClose()
400400
{
401401
_closeMessageReceived = true;
402402

403+
// signal that SSH_MSG_CHANNEL_CLOSE message was received from server
404+
// we need to signal this before firing the Closed event, as a subscriber
405+
// may very well react to the Closed event by closing or disposing the
406+
// channel which in turn will wait for this handle to be signaled
407+
var channelClosedWaitHandle = _channelClosedWaitHandle;
408+
if (channelClosedWaitHandle != null)
409+
channelClosedWaitHandle.Set();
410+
403411
// raise event signaling that the server has closed its end of the channel
404412
var closed = Closed;
405413
if (closed != null)
406414
{
407415
closed(this, new ChannelEventArgs(LocalChannelNumber));
408416
}
409417

410-
// signal that SSH_MSG_CHANNEL_CLOSE message was received from server
411-
var channelClosedWaitHandle = _channelClosedWaitHandle;
412-
if (channelClosedWaitHandle != null)
413-
channelClosedWaitHandle.Set();
414-
415418
// close the channel
416419
Close();
417420
}

0 commit comments

Comments
 (0)