Skip to content

Bugfix/shell stream stopping event on chanell close #1024

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/Renci.SshNet.Tests/Classes/ShellStreamTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ protected override void OnInit()
_sessionMock = new Mock<ISession>(MockBehavior.Strict);
_connectionInfoMock = new Mock<IConnectionInfo>(MockBehavior.Strict);
_channelSessionMock = new Mock<IChannelSession>(MockBehavior.Strict);

}

[TestMethod] // issue #2190
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@ public class SshClientTest_CreateShellStream_TerminalNameAndColumnsAndRowsAndWid
private uint _heightRows;
private uint _widthPixels;
private uint _heightPixels;
private EventHandler<EventArgs> _starting;
private EventHandler<EventArgs> _stopping;
private Dictionary<TerminalModes, uint> _terminalModes;
private int _bufferSize;
private ShellStream _expected;
Expand Down Expand Up @@ -59,7 +61,9 @@ protected override void SetupMocks()
_widthPixels,
_heightPixels,
_terminalModes,
_bufferSize))
_bufferSize,
_starting,
_stopping))
.Returns(_expected);
}

Expand All @@ -79,7 +83,9 @@ protected override void Act()
_widthPixels,
_heightPixels,
_bufferSize,
_terminalModes);
_terminalModes,
_starting,
_stopping);
}

[TestMethod]
Expand All @@ -92,7 +98,9 @@ public void CreateShellStreamOnServiceFactoryShouldBeInvokedOnce()
_widthPixels,
_heightPixels,
_terminalModes,
_bufferSize),
_bufferSize,
_starting,
_stopping),
Times.Once);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ public class SshClientTest_CreateShellStream_TerminalNameAndColumnsAndRowsAndWid
private uint _widthPixels;
private uint _heightPixels;
private int _bufferSize;
private EventHandler<EventArgs> _starting;
private EventHandler<EventArgs> _stopping;
private ShellStream _expected;
private ShellStream _actual;

Expand Down Expand Up @@ -57,7 +59,10 @@ protected override void SetupMocks()
_widthPixels,
_heightPixels,
null,
_bufferSize))
_bufferSize,
_starting,
_stopping
))
.Returns(_expected);
}

Expand Down Expand Up @@ -89,7 +94,9 @@ public void CreateShellStreamOnServiceFactoryShouldBeInvokedOnce()
_widthPixels,
_heightPixels,
null,
_bufferSize),
_bufferSize,
_starting,
_stopping),
Times.Once);
}

Expand Down
14 changes: 14 additions & 0 deletions src/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,20 @@
<MSTestV1UnitTestFrameworkAssemblyCandidate>$(MSBuildProgramFiles32)\Microsoft Visual Studio\2019\Preview\Common7\IDE\PublicAssemblies\Microsoft.VisualStudio.QualityTools.UnitTestFramework.dll</MSTestV1UnitTestFrameworkAssemblyCandidate>
<MSTestV1UnitTestFrameworkAssembly Condition="'$(MSTestV1UnitTestFrameworkAssembly)' == '' and Exists('$(MSTestV1UnitTestFrameworkAssemblyCandidate)')">$(MSTestV1UnitTestFrameworkAssemblyCandidate)</MSTestV1UnitTestFrameworkAssembly>
</PropertyGroup>
<PropertyGroup Condition=" '$(VisualStudioVersion)' == '17.0' ">
<!-- Look for Microsoft.VisualStudio.QualityTools.UnitTestFramework.dll in VS 2019 Enterprise -->
<MSTestV1UnitTestFrameworkAssemblyCandidate>$(MSBuildProgramFiles32)\Microsoft Visual Studio\2022\Enterprise\Common7\IDE\PublicAssemblies\Microsoft.VisualStudio.QualityTools.UnitTestFramework.dll</MSTestV1UnitTestFrameworkAssemblyCandidate>
<MSTestV1UnitTestFrameworkAssembly Condition="'$(MSTestV1UnitTestFrameworkAssembly)' == '' and Exists('$(MSTestV1UnitTestFrameworkAssemblyCandidate)')">$(MSTestV1UnitTestFrameworkAssemblyCandidate)</MSTestV1UnitTestFrameworkAssembly>
<!-- Fall back to VS 2022 Professional -->
<MSTestV1UnitTestFrameworkAssemblyCandidate>$(MSBuildProgramFiles32)\Microsoft Visual Studio\2022\Professional\Common7\IDE\PublicAssemblies\Microsoft.VisualStudio.QualityTools.UnitTestFramework.dll</MSTestV1UnitTestFrameworkAssemblyCandidate>
<MSTestV1UnitTestFrameworkAssembly Condition="'$(MSTestV1UnitTestFrameworkAssembly)' == '' and Exists('$(MSTestV1UnitTestFrameworkAssemblyCandidate)')">$(MSTestV1UnitTestFrameworkAssemblyCandidate)</MSTestV1UnitTestFrameworkAssembly>
<!-- Fall back to VS 2022 Community -->
<MSTestV1UnitTestFrameworkAssemblyCandidate>$(MSBuildProgramFiles32)\Microsoft Visual Studio\2022\Community\Common7\IDE\PublicAssemblies\Microsoft.VisualStudio.QualityTools.UnitTestFramework.dll</MSTestV1UnitTestFrameworkAssemblyCandidate>
<MSTestV1UnitTestFrameworkAssembly Condition="'$(MSTestV1UnitTestFrameworkAssembly)' == '' and Exists('$(MSTestV1UnitTestFrameworkAssemblyCandidate)')">$(MSTestV1UnitTestFrameworkAssemblyCandidate)</MSTestV1UnitTestFrameworkAssembly>
<!-- Fall back to VS 2022 Preview -->
<MSTestV1UnitTestFrameworkAssemblyCandidate>$(MSBuildProgramFiles32)\Microsoft Visual Studio\2022\Preview\Common7\IDE\PublicAssemblies\Microsoft.VisualStudio.QualityTools.UnitTestFramework.dll</MSTestV1UnitTestFrameworkAssemblyCandidate>
<MSTestV1UnitTestFrameworkAssembly Condition="'$(MSTestV1UnitTestFrameworkAssembly)' == '' and Exists('$(MSTestV1UnitTestFrameworkAssemblyCandidate)')">$(MSTestV1UnitTestFrameworkAssemblyCandidate)</MSTestV1UnitTestFrameworkAssembly>
</PropertyGroup>

<ItemGroup Condition="'$(TargetFramework)' == 'net35'">
<Reference Include="Microsoft.VisualStudio.QualityTools.UnitTestFramework">
Expand Down
9 changes: 7 additions & 2 deletions src/Renci.SshNet/IServiceFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,9 @@ internal partial interface IServiceFactory
/// <param name="width">The terminal height in pixels.</param>
/// <param name="height">The terminal height in pixels.</param>
/// <param name="bufferSize">Size of the buffer.</param>
/// <param name="terminalModeValues">The terminal mode values.</param>
/// <param name="terminalModeValues">List of Terminal Modes</param>
/// <param name="starting">Optional Starting Event handler</param>
/// <param name="stopping">Optional Stopping Event handler</param>
/// <returns>
/// The created <see cref="ShellStream"/> instance.
/// </returns>
Expand All @@ -100,7 +102,10 @@ ShellStream CreateShellStream(ISession session,
uint width,
uint height,
IDictionary<TerminalModes, uint> terminalModeValues,
int bufferSize);
int bufferSize,
EventHandler<EventArgs> starting = null,
EventHandler<EventArgs> stopping = null
);

/// <summary>
/// Creates an <see cref="IRemotePathTransformation"/> that encloses a path in double quotes, and escapes
Expand Down
9 changes: 7 additions & 2 deletions src/Renci.SshNet/ServiceFactory.cs
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,9 @@ public ISftpResponseFactory CreateSftpResponseFactory()
/// <param name="height">The terminal height in pixels.</param>
/// <param name="terminalModeValues">The terminal mode values.</param>
/// <param name="bufferSize">The size of the buffer.</param>
/// <param name="starting">Optional Starting Event handler</param>
/// <param name="stopping">Optional Stopping Event handler</param>
///
/// <returns>
/// The created <see cref="ShellStream"/> instance.
/// </returns>
Expand All @@ -175,9 +178,9 @@ public ISftpResponseFactory CreateSftpResponseFactory()
/// to the drawable area of the window.
/// </para>
/// </remarks>
public ShellStream CreateShellStream(ISession session, string terminalName, uint columns, uint rows, uint width, uint height, IDictionary<TerminalModes, uint> terminalModeValues, int bufferSize)
public ShellStream CreateShellStream(ISession session, string terminalName, uint columns, uint rows, uint width, uint height, IDictionary<TerminalModes, uint> terminalModeValues, int bufferSize, EventHandler<EventArgs> starting = null, EventHandler<EventArgs> stopping = null)
{
return new ShellStream(session, terminalName, columns, rows, width, height, terminalModeValues, bufferSize);
return new ShellStream(session, terminalName, columns, rows, width, height, terminalModeValues, bufferSize, starting, stopping);
}

/// <summary>
Expand Down Expand Up @@ -251,5 +254,7 @@ public ISocketFactory CreateSocketFactory()
{
return new SocketFactory();
}


}
}
47 changes: 37 additions & 10 deletions src/Renci.SshNet/ShellStream.cs
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
using System;
using System.Collections.Generic;
using System.Text;
using System.IO;
using Renci.SshNet.Abstractions;
using Renci.SshNet.Channels;
using Renci.SshNet.Common;
using System.Threading;
using System;
using System.Collections.Generic;
using System.IO;
using System.Text;
using System.Text.RegularExpressions;
using Renci.SshNet.Abstractions;
using System.Threading;

namespace Renci.SshNet
{
Expand Down Expand Up @@ -36,6 +36,16 @@ public class ShellStream : Stream
/// </summary>
public event EventHandler<ExceptionEventArgs> ErrorOccurred;

/// <summary>
/// Occurs when the closing channel has requested that the stream close
/// </summary>
public event EventHandler<EventArgs> Stopping;

/// <summary>
/// Occurs when a channel has been established and the stream is about to start
/// </summary>
public event EventHandler<EventArgs> Starting;

/// <summary>
/// Gets a value that indicates whether data is available on the <see cref="ShellStream"/> to be read.
/// </summary>
Expand Down Expand Up @@ -74,11 +84,13 @@ internal int BufferSize
/// <param name="width">The terminal height in pixels.</param>
/// <param name="height">The terminal height in pixels.</param>
/// <param name="terminalModeValues">The terminal mode values.</param>
/// <param name="bufferSize">The size of the buffer.</param>
/// <param name="bufferSize">Size of the buffer.</param>
/// <param name="starting">Optional Starting Event handler</param>
/// <param name="stopping">Optional Stopping Event handler</param>
/// <exception cref="SshException">The channel could not be opened.</exception>
/// <exception cref="SshException">The pseudo-terminal request was not accepted by the server.</exception>
/// <exception cref="SshException">The request to start a shell was not accepted by the server.</exception>
internal ShellStream(ISession session, string terminalName, uint columns, uint rows, uint width, uint height, IDictionary<TerminalModes, uint> terminalModeValues, int bufferSize)
internal ShellStream(ISession session, string terminalName, uint columns, uint rows, uint width, uint height, IDictionary<TerminalModes, uint> terminalModeValues, int bufferSize, EventHandler<EventArgs> starting = null, EventHandler<EventArgs> stopping = null)
{
_encoding = session.ConnectionInfo.Encoding;
_session = session;
Expand All @@ -91,9 +103,14 @@ internal ShellStream(ISession session, string terminalName, uint columns, uint r
_channel.Closed += Channel_Closed;
_session.Disconnected += Session_Disconnected;
_session.ErrorOccured += Session_ErrorOccured;

Starting = starting;
Stopping = stopping;
try
{
if (Starting != null)
{
Starting(this, new EventArgs());
}
_channel.Open();
if (!_channel.SendPseudoTerminalRequest(terminalName, columns, rows, width, height, terminalModeValues))
{
Expand Down Expand Up @@ -772,7 +789,17 @@ private void Session_Disconnected(object sender, EventArgs e)

private void Channel_Closed(object sender, ChannelEventArgs e)
{
// TODO: Do we need to call dispose here ??
if (Stopping != null)
{
ThreadAbstraction.ExecuteThread(() => Stopping(this, new EventArgs()));
}

// FYI: Dispose should Probably NOT be called here as a class should NOT dispose of
// itself: It will be owned by someone else. Calling it here means it will be disappear
// out from under the owner. BUT removing it here now is a breaking change and because there
// is a workaround in the Flush method (raisign exception). I added the notify event
// here instead so that the user knows the channel is no longer usable.
// -- REMOVE NEXT LINE on next major release (## noted 2020.0.0.1)
Dispose();
}

Expand Down
21 changes: 13 additions & 8 deletions src/Renci.SshNet/SshClient.cs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
using System;
using Renci.SshNet.Common;
using System;
using System.Collections.Generic;
using System.IO;
using System.Text;
using System.Diagnostics.CodeAnalysis;
using System.IO;
using System.Net;
using Renci.SshNet.Common;
using System.Text;

namespace Renci.SshNet
{
Expand Down Expand Up @@ -413,6 +413,8 @@ public Shell CreateShell(Encoding encoding, string input, Stream output, Stream
/// <param name="width">The terminal height in pixels.</param>
/// <param name="height">The terminal height in pixels.</param>
/// <param name="bufferSize">The size of the buffer.</param>
/// <param name="starting">Optional Starting Event handler</param>
/// <param name="stopping">Optional Stopping Event handler</param>
/// <returns>
/// The created <see cref="ShellStream"/> instance.
/// </returns>
Expand All @@ -427,9 +429,9 @@ public Shell CreateShell(Encoding encoding, string input, Stream output, Stream
/// to the drawable area of the window.
/// </para>
/// </remarks>
public ShellStream CreateShellStream(string terminalName, uint columns, uint rows, uint width, uint height, int bufferSize)
public ShellStream CreateShellStream(string terminalName, uint columns, uint rows, uint width, uint height, int bufferSize, EventHandler<EventArgs> starting = null, EventHandler<EventArgs> stopping = null)
{
return CreateShellStream(terminalName, columns, rows, width, height, bufferSize, null);
return CreateShellStream(terminalName, columns, rows, width, height, bufferSize, null, starting, stopping);
}

/// <summary>
Expand All @@ -442,6 +444,9 @@ public ShellStream CreateShellStream(string terminalName, uint columns, uint row
/// <param name="height">The terminal height in pixels.</param>
/// <param name="bufferSize">The size of the buffer.</param>
/// <param name="terminalModeValues">The terminal mode values.</param>
/// <param name="starting">Optional Starting Event handler</param>
/// <param name="stopping">Optional Stopping Event handler</param>
///
/// <returns>
/// The created <see cref="ShellStream"/> instance.
/// </returns>
Expand All @@ -456,11 +461,11 @@ public ShellStream CreateShellStream(string terminalName, uint columns, uint row
/// to the drawable area of the window.
/// </para>
/// </remarks>
public ShellStream CreateShellStream(string terminalName, uint columns, uint rows, uint width, uint height, int bufferSize, IDictionary<TerminalModes, uint> terminalModeValues)
public ShellStream CreateShellStream(string terminalName, uint columns, uint rows, uint width, uint height, int bufferSize, IDictionary<TerminalModes, uint> terminalModeValues, EventHandler<EventArgs> starting = null, EventHandler<EventArgs> stopping = null)
{
EnsureSessionIsOpen();

return ServiceFactory.CreateShellStream(Session, terminalName, columns, rows, width, height, terminalModeValues, bufferSize);
return ServiceFactory.CreateShellStream(Session, terminalName, columns, rows, width, height, terminalModeValues, bufferSize, starting, stopping);
}

/// <summary>
Expand Down