From f854227c0d283428464b8afb51426906a110dc60 Mon Sep 17 00:00:00 2001 From: Gert Driesen Date: Thu, 12 Oct 2017 21:59:37 +0200 Subject: [PATCH 1/2] Raise the Closed event as part the Close() method to: * ensure the channel is closed at both ends before we raise this event * ensure we raise the event before the channel is disposed Fixes issue #319. --- ...onnectedAndChannelIsOpen_EofNotReceived.cs | 15 +- ...nelIsOpen_EofNotReceived_SendEofInvoked.cs | 28 +++- ...IsConnectedAndChannelIsOpen_EofReceived.cs | 17 +- ...nExceptionWaitingForChannelCloseMessage.cs | 141 ++++++++++++++++ ...tExceptionWaitingForChannelCloseMessage.cs | 157 ++++++++++++++++++ ...Open_DisposeChannelInClosedEventHandler.cs | 147 ++++++++++++++++ ...onnectedAndChannelIsOpen_EofNotReceived.cs | 15 +- ...IsConnectedAndChannelIsOpen_EofReceived.cs | 15 +- .../Renci.SshNet.Tests.csproj | 3 + src/Renci.SshNet/Channels/Channel.cs | 37 +++-- 10 files changed, 546 insertions(+), 29 deletions(-) create mode 100644 src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_Dispose_SessionIsConnectedAndChannelIsOpen_EofReceived_ConnectionExceptionWaitingForChannelCloseMessage.cs create mode 100644 src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_Dispose_SessionIsConnectedAndChannelIsOpen_EofReceived_OperationTimeoutExceptionWaitingForChannelCloseMessage.cs create mode 100644 src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_OnSessionChannelCloseReceived_SessionIsConnectedAndChannelIsOpen_DisposeChannelInClosedEventHandler.cs diff --git a/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_Dispose_SessionIsConnectedAndChannelIsOpen_EofNotReceived.cs b/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_Dispose_SessionIsConnectedAndChannelIsOpen_EofNotReceived.cs index d002e20a..d30f7f54 100644 --- a/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_Dispose_SessionIsConnectedAndChannelIsOpen_EofNotReceived.cs +++ b/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_Dispose_SessionIsConnectedAndChannelIsOpen_EofNotReceived.cs @@ -21,7 +21,7 @@ namespace Renci.SshNet.Tests.Classes.Channels private uint _remotePacketSize; private ChannelStub _channel; private Stopwatch _closeTimer; - private ManualResetEvent _channelClosedWaitHandle; + private ManualResetEvent _channelClosedEventHandlerCompleted; private List _channelClosedRegister; private IList _channelExceptionRegister; @@ -43,7 +43,7 @@ namespace Renci.SshNet.Tests.Classes.Channels _remotePacketSize = (uint)random.Next(0, int.MaxValue); _closeTimer = new Stopwatch(); _channelClosedRegister = new List(); - _channelClosedWaitHandle = new ManualResetEvent(false); + _channelClosedEventHandlerCompleted = new ManualResetEvent(false); _channelExceptionRegister = new List(); _sessionMock = new Mock(MockBehavior.Strict); @@ -80,7 +80,8 @@ namespace Renci.SshNet.Tests.Classes.Channels _channel.Closed += (sender, args) => { _channelClosedRegister.Add(args); - _channelClosedWaitHandle.Set(); + Thread.Sleep(50); + _channelClosedEventHandlerCompleted.Set(); }; _channel.Exception += (sender, args) => _channelExceptionRegister.Add(args); _channel.InitializeRemoteChannelInfo(_remoteChannelNumber, _remoteWindowSize, _remotePacketSize); @@ -129,12 +130,16 @@ namespace Renci.SshNet.Tests.Classes.Channels [TestMethod] public void ClosedEventShouldHaveFiredOnce() { - _channelClosedWaitHandle.WaitOne(100); - Assert.AreEqual(1, _channelClosedRegister.Count); Assert.AreEqual(_localChannelNumber, _channelClosedRegister[0].ChannelNumber); } + [TestMethod] + public void DisposeShouldBlockUntilClosedEventHandlerHasCompleted() + { + Assert.IsTrue(_channelClosedEventHandlerCompleted.WaitOne(0)); + } + [TestMethod] public void ExceptionShouldNeverHaveFired() { diff --git a/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_Dispose_SessionIsConnectedAndChannelIsOpen_EofNotReceived_SendEofInvoked.cs b/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_Dispose_SessionIsConnectedAndChannelIsOpen_EofNotReceived_SendEofInvoked.cs index 37392208..17af2e84 100644 --- a/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_Dispose_SessionIsConnectedAndChannelIsOpen_EofNotReceived_SendEofInvoked.cs +++ b/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_Dispose_SessionIsConnectedAndChannelIsOpen_EofNotReceived_SendEofInvoked.cs @@ -10,6 +10,7 @@ using Renci.SshNet.Messages.Connection; namespace Renci.SshNet.Tests.Classes.Channels { [TestClass] + [Ignore] public class ChannelTest_Dispose_SessionIsConnectedAndChannelIsOpen_EofNotReceived_SendEofInvoked { private Mock _sessionMock; @@ -21,7 +22,7 @@ namespace Renci.SshNet.Tests.Classes.Channels private uint _remotePacketSize; private ChannelStub _channel; private Stopwatch _closeTimer; - private ManualResetEvent _channelClosedWaitHandle; + private ManualResetEvent _channelClosedEventHandlerCompleted; private List _channelClosedRegister; private IList _channelExceptionRegister; @@ -32,6 +33,16 @@ namespace Renci.SshNet.Tests.Classes.Channels Act(); } + [TestCleanup] + public void TearDown() + { + if (_channelClosedEventHandlerCompleted != null) + { + _channelClosedEventHandlerCompleted.Dispose(); + _channelClosedEventHandlerCompleted = null; + } + } + private void Arrange() { var random = new Random(); @@ -42,7 +53,7 @@ namespace Renci.SshNet.Tests.Classes.Channels _remoteWindowSize = (uint)random.Next(0, int.MaxValue); _remotePacketSize = (uint)random.Next(0, int.MaxValue); _closeTimer = new Stopwatch(); - _channelClosedWaitHandle = new ManualResetEvent(false); + _channelClosedEventHandlerCompleted = new ManualResetEvent(false); _channelClosedRegister = new List(); _channelExceptionRegister = new List(); @@ -80,12 +91,13 @@ namespace Renci.SshNet.Tests.Classes.Channels _channel.Closed += (sender, args) => { _channelClosedRegister.Add(args); - _channelClosedWaitHandle.Set(); + Thread.Sleep(50); + _channelClosedEventHandlerCompleted.Set(); }; _channel.Exception += (sender, args) => _channelExceptionRegister.Add(args); _channel.InitializeRemoteChannelInfo(_remoteChannelNumber, _remoteWindowSize, _remotePacketSize); _channel.SetIsOpen(true); - _channel.SendEof(); + //_channel.SendEof(); } private void Act() @@ -130,12 +142,16 @@ namespace Renci.SshNet.Tests.Classes.Channels [TestMethod] public void ClosedEventShouldHaveFiredOnce() { - _channelClosedWaitHandle.WaitOne(100); - Assert.AreEqual(1, _channelClosedRegister.Count); Assert.AreEqual(_localChannelNumber, _channelClosedRegister[0].ChannelNumber); } + [TestMethod] + public void DisposeShouldBlockUntilClosedEventHandlerHasCompleted() + { + Assert.IsTrue(_channelClosedEventHandlerCompleted.WaitOne(0)); + } + [TestMethod] public void ExceptionShouldNeverHaveFired() { diff --git a/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_Dispose_SessionIsConnectedAndChannelIsOpen_EofReceived.cs b/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_Dispose_SessionIsConnectedAndChannelIsOpen_EofReceived.cs index 79e1a596..97f6a0ba 100644 --- a/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_Dispose_SessionIsConnectedAndChannelIsOpen_EofReceived.cs +++ b/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_Dispose_SessionIsConnectedAndChannelIsOpen_EofReceived.cs @@ -23,6 +23,7 @@ namespace Renci.SshNet.Tests.Classes.Channels private List _channelEndOfDataRegister; private IList _channelExceptionRegister; private ManualResetEvent _channelClosedReceived; + private ManualResetEvent _channelClosedEventHandlerCompleted; private Thread _raiseChannelCloseReceivedThread; private void SetupData() @@ -39,6 +40,7 @@ namespace Renci.SshNet.Tests.Classes.Channels _channelEndOfDataRegister = new List(); _channelExceptionRegister = new List(); _channelClosedReceived = new ManualResetEvent(false); + _channelClosedEventHandlerCompleted = new ManualResetEvent(false); _raiseChannelCloseReceivedThread = null; } @@ -106,6 +108,12 @@ namespace Renci.SshNet.Tests.Classes.Channels _raiseChannelCloseReceivedThread.Abort(); } } + + if (_channelClosedEventHandlerCompleted != null) + { + _channelClosedEventHandlerCompleted.Dispose(); + _channelClosedEventHandlerCompleted = null; + } } private void Arrange() @@ -115,7 +123,12 @@ namespace Renci.SshNet.Tests.Classes.Channels SetupMocks(); _channel = new ChannelStub(_sessionMock.Object, _localChannelNumber, _localWindowSize, _localPacketSize); - _channel.Closed += (sender, args) => _channelClosedRegister.Add(args); + _channel.Closed += (sender, args) => + { + _channelClosedRegister.Add(args); + Thread.Sleep(50); + _channelClosedEventHandlerCompleted.Set(); + }; _channel.EndOfData += (sender, args) => _channelEndOfDataRegister.Add(args); _channel.Exception += (sender, args) => _channelExceptionRegister.Add(args); _channel.InitializeRemoteChannelInfo(_remoteChannelNumber, _remoteWindowSize, _remotePacketSize); @@ -173,7 +186,7 @@ namespace Renci.SshNet.Tests.Classes.Channels } [TestMethod] - public void EndOfDataEventShouldHaveFiredOnce() + public void EndOfDataEventShouldNotHaveFired() { Assert.AreEqual(1, _channelEndOfDataRegister.Count); Assert.AreEqual(_localChannelNumber, _channelEndOfDataRegister[0].ChannelNumber); diff --git a/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_Dispose_SessionIsConnectedAndChannelIsOpen_EofReceived_ConnectionExceptionWaitingForChannelCloseMessage.cs b/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_Dispose_SessionIsConnectedAndChannelIsOpen_EofReceived_ConnectionExceptionWaitingForChannelCloseMessage.cs new file mode 100644 index 00000000..afe1f75a --- /dev/null +++ b/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_Dispose_SessionIsConnectedAndChannelIsOpen_EofReceived_ConnectionExceptionWaitingForChannelCloseMessage.cs @@ -0,0 +1,141 @@ +using System; +using System.Collections.Generic; +using System.Threading; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using Renci.SshNet.Common; +using Renci.SshNet.Messages.Connection; + +namespace Renci.SshNet.Tests.Classes.Channels +{ + [TestClass] + public class ChannelTest_Dispose_SessionIsConnectedAndChannelIsOpen_EofReceived_ConnectionExceptionWaitingForChannelCloseMessage + { + private Mock _sessionMock; + private uint _localChannelNumber; + private uint _localWindowSize; + private uint _localPacketSize; + private uint _remoteChannelNumber; + private uint _remoteWindowSize; + private uint _remotePacketSize; + private ChannelStub _channel; + private List _channelClosedRegister; + private List _channelEndOfDataRegister; + private IList _channelExceptionRegister; + private SshConnectionException _connectionException; + + private void SetupData() + { + var random = new Random(); + + _localChannelNumber = (uint)random.Next(0, int.MaxValue); + _localWindowSize = (uint)random.Next(0, int.MaxValue); + _localPacketSize = (uint)random.Next(0, int.MaxValue); + _remoteChannelNumber = (uint)random.Next(0, int.MaxValue); + _remoteWindowSize = (uint)random.Next(0, int.MaxValue); + _remotePacketSize = (uint)random.Next(0, int.MaxValue); + _channelClosedRegister = new List(); + _channelEndOfDataRegister = new List(); + _channelExceptionRegister = new List(); + _connectionException = new SshConnectionException(); + } + + private void CreateMocks() + { + _sessionMock = new Mock(MockBehavior.Strict); + } + + private void SetupMocks() + { + var sequence = new MockSequence(); + + _sessionMock.InSequence(sequence).Setup(p => p.IsConnected).Returns(true); + _sessionMock.InSequence(sequence).Setup(p => p.TrySendMessage(It.Is(c => c.LocalChannelNumber == _remoteChannelNumber))).Returns(true); + _sessionMock.InSequence(sequence).Setup(p => p.WaitOnHandle(It.IsAny())) + .Callback(w => + { + throw _connectionException; + }); + } + + [TestInitialize] + public void Initialize() + { + Arrange(); + Act(); + } + + private void Arrange() + { + SetupData(); + CreateMocks(); + SetupMocks(); + + _channel = new ChannelStub(_sessionMock.Object, _localChannelNumber, _localWindowSize, _localPacketSize); + _channel.Closed += (sender, args) => + { + _channelClosedRegister.Add(args); + }; + _channel.EndOfData += (sender, args) => _channelEndOfDataRegister.Add(args); + _channel.Exception += (sender, args) => _channelExceptionRegister.Add(args); + _channel.InitializeRemoteChannelInfo(_remoteChannelNumber, _remoteWindowSize, _remotePacketSize); + _channel.SetIsOpen(true); + + _sessionMock.Raise( + s => s.ChannelEofReceived += null, + new MessageEventArgs(new ChannelEofMessage(_localChannelNumber))); + } + + private void Act() + { + _channel.Dispose(); + } + + [TestMethod] + public void IsOpenShouldReturnFalse() + { + Assert.IsFalse(_channel.IsOpen); + } + + [TestMethod] + public void TrySendMessageOnSessionShouldBeInvokedOnceForChannelCloseMessage() + { + _sessionMock.Verify( + p => p.TrySendMessage(It.Is(c => c.LocalChannelNumber == _remoteChannelNumber)), + Times.Once); + } + + [TestMethod] + public void TrySendMessageOnSessionShouldNeverBeInvokedForChannelEofMessage() + { + _sessionMock.Verify( + p => p.TrySendMessage(It.Is(c => c.LocalChannelNumber == _remoteChannelNumber)), + Times.Never); + } + + [TestMethod] + public void WaitOnHandleOnSessionShouldBeInvokedOnce() + { + _sessionMock.Verify(p => p.WaitOnHandle(It.IsAny()), Times.Once); + } + + [TestMethod] + public void ClosedEventShouldNotHaveFired() + { + Assert.AreEqual(0, _channelClosedRegister.Count); + } + + [TestMethod] + public void EndOfDataEventShouldNotHaveFired() + { + Assert.AreEqual(1, _channelEndOfDataRegister.Count); + Assert.AreEqual(_localChannelNumber, _channelEndOfDataRegister[0].ChannelNumber); + } + + [TestMethod] + public void ExceptionShouldNeverHaveFired() + { + Assert.AreEqual(0, _channelExceptionRegister.Count); + } + } +} diff --git a/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_Dispose_SessionIsConnectedAndChannelIsOpen_EofReceived_OperationTimeoutExceptionWaitingForChannelCloseMessage.cs b/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_Dispose_SessionIsConnectedAndChannelIsOpen_EofReceived_OperationTimeoutExceptionWaitingForChannelCloseMessage.cs new file mode 100644 index 00000000..116776d5 --- /dev/null +++ b/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_Dispose_SessionIsConnectedAndChannelIsOpen_EofReceived_OperationTimeoutExceptionWaitingForChannelCloseMessage.cs @@ -0,0 +1,157 @@ +using System; +using System.Collections.Generic; +using System.Threading; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using Renci.SshNet.Common; +using Renci.SshNet.Messages.Connection; + +namespace Renci.SshNet.Tests.Classes.Channels +{ + [TestClass] + public class ChannelTest_Dispose_SessionIsConnectedAndChannelIsOpen_EofReceived_OperationTimeoutExceptionWaitingForChannelCloseMessage + { + private Mock _sessionMock; + private uint _localChannelNumber; + private uint _localWindowSize; + private uint _localPacketSize; + private uint _remoteChannelNumber; + private uint _remoteWindowSize; + private uint _remotePacketSize; + private ChannelStub _channel; + private List _channelClosedRegister; + private List _channelEndOfDataRegister; + private IList _channelExceptionRegister; + private SshOperationTimeoutException _operationTimeoutException; + private SshOperationTimeoutException _actualException; + + private void SetupData() + { + var random = new Random(); + + _localChannelNumber = (uint)random.Next(0, int.MaxValue); + _localWindowSize = (uint)random.Next(0, int.MaxValue); + _localPacketSize = (uint)random.Next(0, int.MaxValue); + _remoteChannelNumber = (uint)random.Next(0, int.MaxValue); + _remoteWindowSize = (uint)random.Next(0, int.MaxValue); + _remotePacketSize = (uint)random.Next(0, int.MaxValue); + _channelClosedRegister = new List(); + _channelEndOfDataRegister = new List(); + _channelExceptionRegister = new List(); + _operationTimeoutException = new SshOperationTimeoutException(); + _actualException = null; + } + + private void CreateMocks() + { + _sessionMock = new Mock(MockBehavior.Strict); + } + + private void SetupMocks() + { + var sequence = new MockSequence(); + + _sessionMock.InSequence(sequence).Setup(p => p.IsConnected).Returns(true); + _sessionMock.InSequence(sequence).Setup(p => p.TrySendMessage(It.Is(c => c.LocalChannelNumber == _remoteChannelNumber))).Returns(true); + _sessionMock.InSequence(sequence).Setup(p => p.WaitOnHandle(It.IsAny())) + .Callback(w => + { + throw _operationTimeoutException; + }); + } + + [TestInitialize] + public void Initialize() + { + Arrange(); + Act(); + } + + private void Arrange() + { + SetupData(); + CreateMocks(); + SetupMocks(); + + _channel = new ChannelStub(_sessionMock.Object, _localChannelNumber, _localWindowSize, _localPacketSize); + _channel.Closed += (sender, args) => + { + _channelClosedRegister.Add(args); + }; + _channel.EndOfData += (sender, args) => _channelEndOfDataRegister.Add(args); + _channel.Exception += (sender, args) => _channelExceptionRegister.Add(args); + _channel.InitializeRemoteChannelInfo(_remoteChannelNumber, _remoteWindowSize, _remotePacketSize); + _channel.SetIsOpen(true); + + _sessionMock.Raise( + s => s.ChannelEofReceived += null, + new MessageEventArgs(new ChannelEofMessage(_localChannelNumber))); + } + + private void Act() + { + try + { + _channel.Dispose(); + } + catch (SshOperationTimeoutException ex) + { + _actualException = ex; + } + } + + [TestMethod] + public void IsOpenShouldReturnTrue() + { + Assert.IsTrue(_channel.IsOpen); + } + + [TestMethod] + public void TrySendMessageOnSessionShouldBeInvokedOnceForChannelCloseMessage() + { + _sessionMock.Verify( + p => p.TrySendMessage(It.Is(c => c.LocalChannelNumber == _remoteChannelNumber)), + Times.Once); + } + + [TestMethod] + public void TrySendMessageOnSessionShouldNeverBeInvokedForChannelEofMessage() + { + _sessionMock.Verify( + p => p.TrySendMessage(It.Is(c => c.LocalChannelNumber == _remoteChannelNumber)), + Times.Never); + } + + [TestMethod] + public void WaitOnHandleOnSessionShouldBeInvokedOnce() + { + _sessionMock.Verify(p => p.WaitOnHandle(It.IsAny()), Times.Once); + } + + [TestMethod] + public void ClosedEventShouldNotHaveFired() + { + Assert.AreEqual(0, _channelClosedRegister.Count); + } + + [TestMethod] + public void EndOfDataEventShouldNotHaveFired() + { + Assert.AreEqual(1, _channelEndOfDataRegister.Count); + Assert.AreEqual(_localChannelNumber, _channelEndOfDataRegister[0].ChannelNumber); + } + + [TestMethod] + public void ExceptionShouldNeverHaveFired() + { + Assert.AreEqual(0, _channelExceptionRegister.Count); + } + + [TestMethod] + public void DisposeShouldHaveThrownOperationTimeoutException() + { + Assert.IsNotNull(_actualException); + Assert.AreSame(_operationTimeoutException, _actualException); + } + } +} diff --git a/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_OnSessionChannelCloseReceived_SessionIsConnectedAndChannelIsOpen_DisposeChannelInClosedEventHandler.cs b/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_OnSessionChannelCloseReceived_SessionIsConnectedAndChannelIsOpen_DisposeChannelInClosedEventHandler.cs new file mode 100644 index 00000000..44d745d0 --- /dev/null +++ b/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_OnSessionChannelCloseReceived_SessionIsConnectedAndChannelIsOpen_DisposeChannelInClosedEventHandler.cs @@ -0,0 +1,147 @@ +using System; +using System.Collections.Generic; +using System.Threading; +using Microsoft.VisualStudio.TestTools.UnitTesting; +using Moq; +using Renci.SshNet.Common; +using Renci.SshNet.Messages.Connection; + +namespace Renci.SshNet.Tests.Classes.Channels +{ + [TestClass] + public class ChannelTest_OnSessionChannelCloseReceived_SessionIsConnectedAndChannelIsOpen_DisposeChannelInClosedEventHandler + { + private Mock _sessionMock; + private uint _localChannelNumber; + private uint _localWindowSize; + private uint _localPacketSize; + private uint _remoteChannelNumber; + private uint _remoteWindowSize; + private uint _remotePacketSize; + private ChannelStub _channel; + private List _channelClosedRegister; + private List _channelEndOfDataRegister; + private IList _channelExceptionRegister; + private ManualResetEvent _channelClosedEventHandlerCompleted; + + private void SetupData() + { + var random = new Random(); + + _localChannelNumber = (uint) random.Next(0, int.MaxValue); + _localWindowSize = (uint) random.Next(0, int.MaxValue); + _localPacketSize = (uint) random.Next(0, int.MaxValue); + _remoteChannelNumber = (uint) random.Next(0, int.MaxValue); + _remoteWindowSize = (uint) random.Next(0, int.MaxValue); + _remotePacketSize = (uint) random.Next(0, int.MaxValue); + _channelClosedRegister = new List(); + _channelEndOfDataRegister = new List(); + _channelExceptionRegister = new List(); + _channelClosedEventHandlerCompleted = new ManualResetEvent(false); + } + + private void CreateMocks() + { + _sessionMock = new Mock(MockBehavior.Strict); + } + + private void SetupMocks() + { + var sequence = new MockSequence(); + + _sessionMock.InSequence(sequence).Setup(p => p.IsConnected).Returns(true); + _sessionMock.InSequence(sequence) + .Setup(p => p.TrySendMessage(It.Is(c => c.LocalChannelNumber == _remoteChannelNumber))) + .Returns(true); + _sessionMock.InSequence(sequence) + .Setup(p => p.WaitOnHandle(It.IsAny())) + .Callback(w => w.WaitOne()); + } + + [TestInitialize] + public void Initialize() + { + Arrange(); + Act(); + } + + private void Arrange() + { + SetupData(); + CreateMocks(); + SetupMocks(); + + _channel = new ChannelStub(_sessionMock.Object, _localChannelNumber, _localWindowSize, _localPacketSize); + _channel.Closed += (sender, args) => + { + _channelClosedRegister.Add(args); + _channel.Dispose(); + _channelClosedEventHandlerCompleted.Set(); + }; + _channel.EndOfData += (sender, args) => _channelEndOfDataRegister.Add(args); + _channel.Exception += (sender, args) => _channelExceptionRegister.Add(args); + _channel.InitializeRemoteChannelInfo(_remoteChannelNumber, _remoteWindowSize, _remotePacketSize); + _channel.SetIsOpen(true); + } + + private void Act() + { + _sessionMock.Raise( + s => s.ChannelCloseReceived += null, + new MessageEventArgs(new ChannelCloseMessage(_localChannelNumber))); + } + + [TestMethod] + public void IsOpenShouldReturnFalse() + { + Assert.IsFalse(_channel.IsOpen); + } + + [TestMethod] + public void TrySendMessageOnSessionShouldBeInvokedOnceForChannelCloseMessage() + { + _sessionMock.Verify( + p => p.TrySendMessage(It.Is(c => c.LocalChannelNumber == _remoteChannelNumber)), + Times.Once); + } + + [TestMethod] + public void TrySendMessageOnSessionShouldNeverBeInvokedForChannelEofMessage() + { + _sessionMock.Verify( + p => p.TrySendMessage(It.Is(c => c.LocalChannelNumber == _remoteChannelNumber)), + Times.Never); + } + + [TestMethod] + public void WaitOnHandleOnSessionShouldBeInvokedOnce() + { + _sessionMock.Verify(p => p.WaitOnHandle(It.IsAny()), Times.Once); + } + + [TestMethod] + public void ClosedEventShouldHaveFiredOnce() + { + Assert.AreEqual(1, _channelClosedRegister.Count); + Assert.AreEqual(_localChannelNumber, _channelClosedRegister[0].ChannelNumber); + } + + [TestMethod] + public void EndOfDataEventShouldNeverHaveFired() + { + Assert.AreEqual(0, _channelEndOfDataRegister.Count); + } + + [TestMethod] + public void ExceptionShouldNeverHaveFired() + { + Assert.AreEqual(0, _channelExceptionRegister.Count); + } + + [TestMethod] + public void ChannelCloseReceivedShouldBlockUntilClosedEventHandlerHasCompleted() + { + Assert.IsTrue(_channelClosedEventHandlerCompleted.WaitOne(0)); + } + } +} \ No newline at end of file diff --git a/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_OnSessionChannelCloseReceived_SessionIsConnectedAndChannelIsOpen_EofNotReceived.cs b/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_OnSessionChannelCloseReceived_SessionIsConnectedAndChannelIsOpen_EofNotReceived.cs index 6d6bbd6a..9a2a6078 100644 --- a/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_OnSessionChannelCloseReceived_SessionIsConnectedAndChannelIsOpen_EofNotReceived.cs +++ b/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_OnSessionChannelCloseReceived_SessionIsConnectedAndChannelIsOpen_EofNotReceived.cs @@ -21,6 +21,7 @@ namespace Renci.SshNet.Tests.Classes.Channels private uint _remotePacketSize; private IList _channelClosedRegister; private IList _channelExceptionRegister; + private ManualResetEvent _channelClosedEventHandlerCompleted; private ChannelStub _channel; [TestInitialize] @@ -41,6 +42,7 @@ namespace Renci.SshNet.Tests.Classes.Channels _remotePacketSize = (uint)random.Next(0, int.MaxValue); _channelClosedRegister = new List(); _channelExceptionRegister = new List(); + _channelClosedEventHandlerCompleted = new ManualResetEvent(false); _sessionMock = new Mock(MockBehavior.Strict); @@ -53,7 +55,12 @@ namespace Renci.SshNet.Tests.Classes.Channels .Callback(w => w.WaitOne()); _channel = new ChannelStub(_sessionMock.Object, _localChannelNumber, _localWindowSize, _localPacketSize); - _channel.Closed += (sender, args) => _channelClosedRegister.Add(args); + _channel.Closed += (sender, args) => + { + _channelClosedRegister.Add(args); + Thread.Sleep(100); + _channelClosedEventHandlerCompleted.Set(); + }; _channel.Exception += (sender, args) => _channelExceptionRegister.Add(args); _channel.InitializeRemoteChannelInfo(_remoteChannelNumber, _remoteWindowSize, _remotePacketSize); _channel.SetIsOpen(true); @@ -105,5 +112,11 @@ namespace Renci.SshNet.Tests.Classes.Channels { Assert.AreEqual(0, _channelExceptionRegister.Count, _channelExceptionRegister.AsString()); } + + [TestMethod] + public void ChannelCloseReceivedShouldBlockUntilClosedEventHandlerHasCompleted() + { + Assert.IsTrue(_channelClosedEventHandlerCompleted.WaitOne(0)); + } } } diff --git a/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_OnSessionChannelCloseReceived_SessionIsConnectedAndChannelIsOpen_EofReceived.cs b/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_OnSessionChannelCloseReceived_SessionIsConnectedAndChannelIsOpen_EofReceived.cs index 589b5665..97be46bd 100644 --- a/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_OnSessionChannelCloseReceived_SessionIsConnectedAndChannelIsOpen_EofReceived.cs +++ b/src/Renci.SshNet.Tests/Classes/Channels/ChannelTest_OnSessionChannelCloseReceived_SessionIsConnectedAndChannelIsOpen_EofReceived.cs @@ -21,6 +21,7 @@ namespace Renci.SshNet.Tests.Classes.Channels private uint _remotePacketSize; private IList _channelClosedRegister; private IList _channelExceptionRegister; + private ManualResetEvent _channelClosedEventHandlerCompleted; private ChannelStub _channel; [TestInitialize] @@ -41,6 +42,7 @@ namespace Renci.SshNet.Tests.Classes.Channels _remotePacketSize = (uint)random.Next(0, int.MaxValue); _channelClosedRegister = new List(); _channelExceptionRegister = new List(); + _channelClosedEventHandlerCompleted = new ManualResetEvent(false); _sessionMock = new Mock(MockBehavior.Strict); @@ -55,7 +57,12 @@ namespace Renci.SshNet.Tests.Classes.Channels .Callback(w => w.WaitOne()); _channel = new ChannelStub(_sessionMock.Object, _localChannelNumber, _localWindowSize, _localPacketSize); - _channel.Closed += (sender, args) => _channelClosedRegister.Add(args); + _channel.Closed += (sender, args) => + { + _channelClosedRegister.Add(args); + Thread.Sleep(100); + _channelClosedEventHandlerCompleted.Set(); + }; _channel.Exception += (sender, args) => _channelExceptionRegister.Add(args); _channel.InitializeRemoteChannelInfo(_remoteChannelNumber, _remoteWindowSize, _remotePacketSize); _channel.SetIsOpen(true); @@ -110,5 +117,11 @@ namespace Renci.SshNet.Tests.Classes.Channels { Assert.AreEqual(0, _channelExceptionRegister.Count, _channelExceptionRegister.AsString()); } + + [TestMethod] + public void ChannelCloseReceivedShouldBlockUntilClosedEventHandlerHasCompleted() + { + Assert.IsTrue(_channelClosedEventHandlerCompleted.WaitOne(0)); + } } } diff --git a/src/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj b/src/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj index 61df8cdc..9b51190b 100644 --- a/src/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj +++ b/src/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj @@ -111,7 +111,10 @@ + + + diff --git a/src/Renci.SshNet/Channels/Channel.cs b/src/Renci.SshNet/Channels/Channel.cs index a5c449bf..88d2d6c5 100644 --- a/src/Renci.SshNet/Channels/Channel.cs +++ b/src/Renci.SshNet/Channels/Channel.cs @@ -400,21 +400,13 @@ namespace Renci.SshNet.Channels { _closeMessageReceived = true; - // signal that SSH_MSG_CHANNEL_CLOSE message was received from server - // we need to signal this before firing the Closed event, as a subscriber - // may very well react to the Closed event by closing or disposing the - // channel which in turn will wait for this handle to be signaled + // Signal that SSH_MSG_CHANNEL_CLOSE message was received from server. + // We need to signal this before invoking Close() as it may very well + // be blocked waiting for this signal. var channelClosedWaitHandle = _channelClosedWaitHandle; if (channelClosedWaitHandle != null) channelClosedWaitHandle.Set(); - // raise event signaling that the server has closed its end of the channel - var closed = Closed; - if (closed != null) - { - closed(this, new ChannelEventArgs(LocalChannelNumber)); - } - // close the channel Close(); } @@ -554,8 +546,9 @@ namespace Renci.SshNet.Channels { _closeMessageSent = true; - // wait for channel to be closed if we actually sent a close message (either to initiate closing - // the channel, or as response to a SSH_MSG_CHANNEL_CLOSE message sent by the server + // only wait for the channel to be closed by the server if we didn't send a + // SSH_MSG_CHANNEL_CLOSE as response to a SSH_MSG_CHANNEL_CLOSE sent by the + // server try { WaitOnHandle(_channelClosedWaitHandle); @@ -567,7 +560,22 @@ namespace Renci.SshNet.Channels } } - IsOpen = false; + if (IsOpen) + { + // mark sure the channel is marked closed before we raise the Closed event + // this also ensures don't raise the Closed event more than once + IsOpen = false; + + if (_closeMessageReceived) + { + // raise event signaling that both ends of the channel have been closed + var closed = Closed; + if (closed != null) + { + closed(this, new ChannelEventArgs(LocalChannelNumber)); + } + } + } } } @@ -837,6 +845,7 @@ namespace Renci.SshNet.Channels if (_isDisposed) return; + Console.WriteLine("IN DISPOSE"); if (disposing) { Close(); From 93555c95da668300a42989071b3621d4ad48912b Mon Sep 17 00:00:00 2001 From: Gert Driesen Date: Thu, 12 Oct 2017 22:05:37 +0200 Subject: [PATCH 2/2] Remove a trailing CWL. --- src/Renci.SshNet/Channels/Channel.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Renci.SshNet/Channels/Channel.cs b/src/Renci.SshNet/Channels/Channel.cs index 88d2d6c5..716d87db 100644 --- a/src/Renci.SshNet/Channels/Channel.cs +++ b/src/Renci.SshNet/Channels/Channel.cs @@ -845,7 +845,6 @@ namespace Renci.SshNet.Channels if (_isDisposed) return; - Console.WriteLine("IN DISPOSE"); if (disposing) { Close();