Dispose channel on EndExecute. Fixes issue #2295.

Add encoding argument to SshCommand ctor.

Renci.SshClient/Build/nuget/SSH.NET.nuspec

@@ -25,6 +25,7 @@ Fixes:
     * SftpClient.Exists(string) returns true for a path that does not exist (issues #1952, #1696 and #1574)
     * ObjectDisposedException when channel is closing (issues #1942 and #1944)
     * Stack overflow during authentication when server returns same allowed methods after partial success (issue #2399)
+    * SshCommand doesn't cleanup subscribed events (issue #2295)
 
 2014.4.6-beta1
 ==============

Renci.SshClient/Renci.SshNet.Tests/Classes/SshCommandTest.cs

@@ -484,7 +484,8 @@ namespace Renci.SshNet.Tests.Classes
         {
             Session session = null; // TODO: Initialize to an appropriate value
             string commandText = string.Empty; // TODO: Initialize to an appropriate value
-            SshCommand target = new SshCommand(session, commandText); // TODO: Initialize to an appropriate value
+            var encoding = Encoding.UTF8;
+            SshCommand target = new SshCommand(session, commandText, encoding); // TODO: Initialize to an appropriate value
             string commandText1 = string.Empty; // TODO: Initialize to an appropriate value
             AsyncCallback callback = null; // TODO: Initialize to an appropriate value
             object state = null; // TODO: Initialize to an appropriate value
@@ -503,7 +504,8 @@ namespace Renci.SshNet.Tests.Classes
         {
             Session session = null; // TODO: Initialize to an appropriate value
             string commandText = string.Empty; // TODO: Initialize to an appropriate value
-            SshCommand target = new SshCommand(session, commandText); // TODO: Initialize to an appropriate value
+            var encoding = Encoding.UTF8;
+            SshCommand target = new SshCommand(session, commandText, encoding); // TODO: Initialize to an appropriate value
             target.CancelAsync();
             Assert.Inconclusive("A method that does not return a value cannot be verified.");
         }
@@ -516,7 +518,8 @@ namespace Renci.SshNet.Tests.Classes
         {
             Session session = null; // TODO: Initialize to an appropriate value
             string commandText = string.Empty; // TODO: Initialize to an appropriate value
-            SshCommand target = new SshCommand(session, commandText); // TODO: Initialize to an appropriate value
+            var encoding = Encoding.UTF8;
+            SshCommand target = new SshCommand(session, commandText, encoding); // TODO: Initialize to an appropriate value
             target.Dispose();
             Assert.Inconclusive("A method that does not return a value cannot be verified.");
         }
@@ -529,7 +532,8 @@ namespace Renci.SshNet.Tests.Classes
         {
             Session session = null; // TODO: Initialize to an appropriate value
             string commandText = string.Empty; // TODO: Initialize to an appropriate value
-            SshCommand target = new SshCommand(session, commandText); // TODO: Initialize to an appropriate value
+            var encoding = Encoding.UTF8;
+            SshCommand target = new SshCommand(session, commandText, encoding); // TODO: Initialize to an appropriate value
             IAsyncResult asyncResult = null; // TODO: Initialize to an appropriate value
             string expected = string.Empty; // TODO: Initialize to an appropriate value
             string actual;
@@ -546,7 +550,8 @@ namespace Renci.SshNet.Tests.Classes
         {
             Session session = null; // TODO: Initialize to an appropriate value
             string commandText = string.Empty; // TODO: Initialize to an appropriate value
-            SshCommand target = new SshCommand(session, commandText); // TODO: Initialize to an appropriate value
+            var encoding = Encoding.UTF8;
+            SshCommand target = new SshCommand(session, commandText, encoding); // TODO: Initialize to an appropriate value
             string expected = string.Empty; // TODO: Initialize to an appropriate value
             string actual;
             actual = target.Execute();
@@ -562,7 +567,8 @@ namespace Renci.SshNet.Tests.Classes
         {
             Session session = null; // TODO: Initialize to an appropriate value
             string commandText = string.Empty; // TODO: Initialize to an appropriate value
-            SshCommand target = new SshCommand(session, commandText); // TODO: Initialize to an appropriate value
+            var encoding = Encoding.UTF8;
+            SshCommand target = new SshCommand(session, commandText, encoding); // TODO: Initialize to an appropriate value
             string commandText1 = string.Empty; // TODO: Initialize to an appropriate value
             string expected = string.Empty; // TODO: Initialize to an appropriate value
             string actual;
@@ -579,7 +585,8 @@ namespace Renci.SshNet.Tests.Classes
         {
             Session session = null; // TODO: Initialize to an appropriate value
             string commandText = string.Empty; // TODO: Initialize to an appropriate value
-            SshCommand target = new SshCommand(session, commandText); // TODO: Initialize to an appropriate value
+            var encoding = Encoding.UTF8;
+            SshCommand target = new SshCommand(session, commandText, encoding); // TODO: Initialize to an appropriate value
             TimeSpan expected = new TimeSpan(); // TODO: Initialize to an appropriate value
             TimeSpan actual;
             target.CommandTimeout = expected;
@@ -596,7 +603,8 @@ namespace Renci.SshNet.Tests.Classes
         {
             Session session = null; // TODO: Initialize to an appropriate value
             string commandText = string.Empty; // TODO: Initialize to an appropriate value
-            SshCommand target = new SshCommand(session, commandText); // TODO: Initialize to an appropriate value
+            var encoding = Encoding.UTF8;
+            SshCommand target = new SshCommand(session, commandText, encoding); // TODO: Initialize to an appropriate value
             string actual;
             actual = target.Error;
             Assert.Inconclusive("Verify the correctness of this test method.");
@@ -610,7 +618,8 @@ namespace Renci.SshNet.Tests.Classes
         {
             Session session = null; // TODO: Initialize to an appropriate value
             string commandText = string.Empty; // TODO: Initialize to an appropriate value
-            SshCommand target = new SshCommand(session, commandText); // TODO: Initialize to an appropriate value
+            var encoding = Encoding.UTF8;
+            SshCommand target = new SshCommand(session, commandText, encoding); // TODO: Initialize to an appropriate value
             string actual;
             actual = target.Result;
             Assert.Inconclusive("Verify the correctness of this test method.");

Renci.SshClient/Renci.SshNet.Tests/Classes/SshCommandTest_EndExecute.cs

@@ -0,0 +1,73 @@
+using System;
+using System.Globalization;
+using System.Text;
+using Microsoft.VisualStudio.TestTools.UnitTesting;
+using Moq;
+using Renci.SshNet.Channels;
+using Renci.SshNet.Common;
+using Renci.SshNet.Tests.Common;
+
+namespace Renci.SshNet.Tests.Classes
+{
+    [TestClass]
+    public class SShCommand_EndExecute : TestBase
+    {
+        private Mock<ISession> _sessionMock;
+        private Mock<IChannelSession> _channelSessionMock;
+        private string _commandText;
+        private Encoding _encoding;
+        private SshCommand _sshCommand;
+
+        protected override void OnInit()
+        {
+            base.OnInit();
+
+            _sessionMock = new Mock<ISession>(MockBehavior.Strict);
+            _commandText = new Random().Next().ToString(CultureInfo.InvariantCulture);
+            _encoding = Encoding.UTF8;
+            _channelSessionMock = new Mock<IChannelSession>(MockBehavior.Strict);
+
+            _sshCommand = new SshCommand(_sessionMock.Object, _commandText, _encoding);
+        }
+
+        [TestMethod]
+        public void EndExecute_ChannelClosed_ShouldDisposeChannelSession()
+        {
+            var seq = new MockSequence();
+
+            _sessionMock.InSequence(seq).Setup(p => p.CreateChannelSession()).Returns(_channelSessionMock.Object);
+            _channelSessionMock.InSequence(seq).Setup(p => p.Open());
+            _channelSessionMock.InSequence(seq).Setup(p => p.SendExecRequest(_commandText))
+                .Returns(true)
+                .Raises(c => c.Closed += null, new ChannelEventArgs(5));
+            _channelSessionMock.InSequence(seq).Setup(p => p.IsOpen).Returns(false);
+            _channelSessionMock.InSequence(seq).Setup(p => p.Dispose());
+
+            var asyncResult = _sshCommand.BeginExecute();
+            _sshCommand.EndExecute(asyncResult);
+
+            _channelSessionMock.Verify(p => p.Dispose(), Times.Once);
+        }
+
+        [TestMethod]
+        public void EndExecute_ChannelOpen_ShouldSendEofAndCloseAndDisposeChannelSession()
+        {
+            var seq = new MockSequence();
+
+            _sessionMock.InSequence(seq).Setup(p => p.CreateChannelSession()).Returns(_channelSessionMock.Object);
+            _channelSessionMock.InSequence(seq).Setup(p => p.Open());
+            _channelSessionMock.InSequence(seq).Setup(p => p.SendExecRequest(_commandText))
+                .Returns(true)
+                .Raises(c => c.Closed += null, new ChannelEventArgs(5));
+            _channelSessionMock.InSequence(seq).Setup(p => p.IsOpen).Returns(true);
+            _channelSessionMock.InSequence(seq).Setup(p => p.SendEof());
+            _channelSessionMock.InSequence(seq).Setup(p => p.Close());
+            _channelSessionMock.InSequence(seq).Setup(p => p.Dispose());
+
+            var asyncResult = _sshCommand.BeginExecute();
+            _sshCommand.EndExecute(asyncResult);
+
+            _channelSessionMock.Verify(p => p.Close(), Times.Once);
+        }
+    }
+}

Renci.SshClient/Renci.SshNet.Tests/Renci.SshNet.Tests.csproj

@@ -240,6 +240,7 @@
     <Compile Include="Classes\ForwardedPortLocalTest.NET40.cs" />
     <Compile Include="Classes\SshCommandTest.NET40.cs" />
     <Compile Include="Classes\ScpClientTest.NET40.cs" />
+    <Compile Include="Classes\SshCommandTest_EndExecute.cs" />
     <Compile Include="Common\AsyncSocketListener.cs" />
     <Compile Include="Common\HttpProxyStub.cs" />
     <Compile Include="Common\HttpRequest.cs" />

Renci.SshClient/Renci.SshNet/Properties/AssemblyInfo.cs

@@ -4,4 +4,5 @@ using System.Runtime.CompilerServices;
 
 [assembly: AssemblyTitle("SSH.NET .NET 4.0")]
 [assembly: Guid("ad816c5e-6f13-4589-9f3e-59523f8b77a4")]
-[assembly: InternalsVisibleTo("Renci.SshNet.Tests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f9194e1eb66b7e2575aaee115ee1d27bc100920e7150e43992d6f668f9737de8b9c7ae892b62b8a36dd1d57929ff1541665d101dc476d6e02390846efae7e5186eec409710fdb596e3f83740afef0d4443055937649bc5a773175b61c57615dac0f0fd10f52b52fedf76c17474cc567b3f7a79de95dde842509fb39aaf69c6c2")]
+[assembly: InternalsVisibleTo("Renci.SshNet.Tests, PublicKey=0024000004800000940000000602000000240000525341310004000001000100f9194e1eb66b7e2575aaee115ee1d27bc100920e7150e43992d6f668f9737de8b9c7ae892b62b8a36dd1d57929ff1541665d101dc476d6e02390846efae7e5186eec409710fdb596e3f83740afef0d4443055937649bc5a773175b61c57615dac0f0fd10f52b52fedf76c17474cc567b3f7a79de95dde842509fb39aaf69c6c2")]
+[assembly: InternalsVisibleTo("DynamicProxyGenAssembly2, PublicKey=0024000004800000940000000602000000240000525341310004000001000100c547cac37abd99c8db225ef2f6c8a3602f3b3606cc9891605d02baa56104f4cfc0734aa39b93bf7852f7d9266654753cc297e7d2edfe0bac1cdcf9f717241550e0a7b191195b7667bb4f64bcb8e2121380fd1d9d46ad2d92d2d15605093924cceaf74c4861eff62abf69b9291ed0a340e113be11e6a7d3113e92484cf7045cc7")]

Renci.SshClient/Renci.SshNet/SshClient.cs

@@ -213,7 +213,7 @@ namespace Renci.SshNet
         public SshCommand CreateCommand(string commandText, Encoding encoding)
         {
             this.ConnectionInfo.Encoding = encoding;
-            return new SshCommand(this.Session, commandText);
+            return new SshCommand(this.Session, commandText, encoding);
         }
 
         /// <summary>

Renci.SshClient/Renci.SshNet/SshCommand.cs

@@ -16,9 +16,10 @@ namespace Renci.SshNet
     /// </summary>
     public partial class SshCommand : IDisposable
     {
-        private readonly Session _session;
+        private readonly ISession _session;
+        private readonly Encoding _encoding;
 
-        private ChannelSession _channel;
+        private IChannelSession _channel;
 
         private CommandAsyncResult _asyncResult;
 
@@ -90,7 +91,7 @@ namespace Renci.SshNet
 
                 if (this.OutputStream != null && this.OutputStream.Length > 0)
                 {
-                    using (var sr = new StreamReader(this.OutputStream, this._session.ConnectionInfo.Encoding))
+                    using (var sr = new StreamReader(this.OutputStream, _encoding))
                     {
                         this._result.Append(sr.ReadToEnd());
                     }
@@ -120,7 +121,7 @@ namespace Renci.SshNet
 
                     if (this.ExtendedOutputStream != null && this.ExtendedOutputStream.Length > 0)
                     {
-                        using (var sr = new StreamReader(this.ExtendedOutputStream, this._session.ConnectionInfo.Encoding))
+                        using (var sr = new StreamReader(this.ExtendedOutputStream, _encoding))
                         {
                             this._error.Append(sr.ReadToEnd());
                         }
@@ -137,16 +138,20 @@ namespace Renci.SshNet
         /// </summary>
         /// <param name="session">The session.</param>
         /// <param name="commandText">The command text.</param>
+        /// <param name="encoding">The encoding to use for the results.</param>
         /// <exception cref="ArgumentNullException">Either <paramref name="session"/>, <paramref name="commandText"/> is null.</exception>
-        internal SshCommand(Session session, string commandText)
+        internal SshCommand(ISession session, string commandText, Encoding encoding)
         {
             if (session == null)
                 throw new ArgumentNullException("session");
             if (commandText == null)
                 throw new ArgumentNullException("commandText");
+            if (encoding == null)
+                throw new ArgumentNullException("encoding");
 
             this._session = session;
             this.CommandText = commandText;
+            this._encoding = encoding;
             this.CommandTimeout = new TimeSpan(0, 0, 0, 0, -1);
 
             this._session.Disconnected += Session_Disconnected;
@@ -246,7 +251,7 @@ namespace Renci.SshNet
         }
 
         /// <summary>
-        /// Begins an asynchronous command execution. 22
+        /// Begins an asynchronous command execution.
         /// </summary>
         /// <param name="commandText">The command text.</param>
         /// <param name="callback">An optional asynchronous callback, to be called when the command execution is complete.</param>
@@ -271,7 +276,6 @@ namespace Renci.SshNet
         /// <example>
         ///     <code source="..\..\Renci.SshNet.Tests\Classes\SshCommandTest.cs" region="Example SshCommand CreateCommand BeginExecute IsCompleted EndExecute" language="C#" title="Asynchronous Command Execution" />
         /// </example>
-        /// <exception cref="System.ArgumentException">Either the IAsyncResult object did not come from the corresponding async method on this type, or EndExecute was called multiple times with the same IAsyncResult.</exception>
         /// <exception cref="ArgumentException">Either the IAsyncResult object did not come from the corresponding async method on this type, or EndExecute was called multiple times with the same IAsyncResult.</exception>
         public string EndExecute(IAsyncResult asyncResult)
         {
@@ -284,16 +288,16 @@ namespace Renci.SshNet
                         //  Make sure that operation completed if not wait for it to finish
                         this.WaitOnHandle(this._asyncResult.AsyncWaitHandle);
 
-                        if (this._channel.IsOpen)
+                        if (_channel.IsOpen)
                         {
-                            this._channel.SendEof();
-
-                            this._channel.Close();
+                            _channel.SendEof();
+                            _channel.Close();
                         }
 
-                        this._channel = null;
+                        _channel.Dispose();
+                        _channel = null;
 
-                        this._asyncResult = null;
+                        _asyncResult = null;
 
                         return this.Result;
                     }
@@ -346,7 +350,7 @@ namespace Renci.SshNet
 
         private void CreateChannel()
         {
-            this._channel = this._session.CreateClientChannel<ChannelSession>();
+            this._channel = this._session.CreateChannelSession();
             this._channel.DataReceived += Channel_DataReceived;
             this._channel.ExtendedDataReceived += Channel_ExtendedDataReceived;
             this._channel.RequestReceived += Channel_RequestReceived;
@@ -419,16 +423,20 @@ namespace Renci.SshNet
 
         private void Channel_RequestReceived(object sender, ChannelRequestEventArgs e)
         {
-            Message replyMessage = new ChannelFailureMessage(this._channel.LocalChannelNumber);
+            Message replyMessage;
 
             if (e.Info is ExitStatusRequestInfo)
             {
                 var exitStatusInfo = e.Info as ExitStatusRequestInfo;
 
-                this.ExitStatus = (int)exitStatusInfo.ExitStatus;
+                this.ExitStatus = (int) exitStatusInfo.ExitStatus;
 
                 replyMessage = new ChannelSuccessMessage(this._channel.LocalChannelNumber);
             }
+            else
+            {
+                replyMessage = new ChannelFailureMessage(this._channel.LocalChannelNumber);
+            }
 
             if (e.Info.WantReply)
             {
@@ -498,7 +506,6 @@ namespace Renci.SshNet
         public void Dispose()
         {
             Dispose(true);
-
             GC.SuppressFinalize(this);
         }
 
@@ -515,7 +522,6 @@ namespace Renci.SshNet
                 // and unmanaged ResourceMessages.
                 if (disposing)
                 {
-
                     this._session.Disconnected -= Session_Disconnected;
                     this._session.ErrorOccured -= Session_ErrorOccured;
 
@@ -543,11 +549,6 @@ namespace Renci.SshNet
                     // Dispose managed ResourceMessages.
                     if (this._channel != null)
                     {
-                        this._channel.DataReceived -= Channel_DataReceived;
-                        this._channel.ExtendedDataReceived -= Channel_ExtendedDataReceived;
-                        this._channel.RequestReceived -= Channel_RequestReceived;
-                        this._channel.Closed -= Channel_Closed;
-
                         this._channel.Dispose();
                         this._channel = null;
                     }