From 8934b5d1566dfca13ab67aeaabcf9727d05285b5 Mon Sep 17 00:00:00 2001 From: olegkap_cp Date: Thu, 7 Feb 2013 19:47:47 +0000 Subject: [PATCH] Improve internal server window size managment Fix exception handeling when multiple threads share one channel that can caus application hanging Improve cipher perfomance Other fixes --- .../ChannelExtendedDataMessageTest.cs | 2 +- .../Classes/SshClientTest.cs | 1 + .../Renci.SshNet/Channels/Channel.cs | 76 ++++++++++++++----- .../Connection/ChannelExtendedDataMessage.cs | 4 +- Renci.SshClient/Renci.SshNet/Session.cs | 8 +- Renci.SshClient/Renci.SshNet/Sftp/SftpFile.cs | 4 + .../Renci.SshNet/Sftp/SftpFileStream.cs | 3 + Renci.SshClient/Renci.SshNet/SftpClient.cs | 72 +++++++++++++----- Renci.SshClient/Renci.SshNet/SshCommand.cs | 6 +- .../Renci.SshNet/SubsystemSession.cs | 12 +-- 10 files changed, 127 insertions(+), 61 deletions(-) diff --git a/Renci.SshClient/Renci.SshNet.Tests/Classes/Messages/Connection/ChannelExtendedDataMessageTest.cs b/Renci.SshClient/Renci.SshNet.Tests/Classes/Messages/Connection/ChannelExtendedDataMessageTest.cs index be3714df..c111bf9b 100644 --- a/Renci.SshClient/Renci.SshNet.Tests/Classes/Messages/Connection/ChannelExtendedDataMessageTest.cs +++ b/Renci.SshClient/Renci.SshNet.Tests/Classes/Messages/Connection/ChannelExtendedDataMessageTest.cs @@ -29,7 +29,7 @@ namespace Renci.SshNet.Tests.Classes.Messages.Connection public void ChannelExtendedDataMessageConstructorTest1() { uint localChannelNumber = 0; // TODO: Initialize to an appropriate value - ChannelExtendedDataMessage target = new ChannelExtendedDataMessage(localChannelNumber); + //ChannelExtendedDataMessage target = new ChannelExtendedDataMessage(localChannelNumber, null, null); Assert.Inconclusive("TODO: Implement code to verify target"); } } diff --git a/Renci.SshClient/Renci.SshNet.Tests/Classes/SshClientTest.cs b/Renci.SshClient/Renci.SshNet.Tests/Classes/SshClientTest.cs index 301cf881..a164fef3 100644 --- a/Renci.SshClient/Renci.SshNet.Tests/Classes/SshClientTest.cs +++ b/Renci.SshClient/Renci.SshNet.Tests/Classes/SshClientTest.cs @@ -49,6 +49,7 @@ namespace Renci.SshNet.Tests.Classes client.HostKeyReceived += delegate(object sender, HostKeyEventArgs e) { hostKeyValidated = true; + if (e.FingerPrint.SequenceEqual(new byte[] { 0x00, 0x01, 0x02, 0x03 })) { e.CanTrust = true; diff --git a/Renci.SshClient/Renci.SshNet/Channels/Channel.cs b/Renci.SshClient/Renci.SshNet/Channels/Channel.cs index ff95afbb..b79d7241 100644 --- a/Renci.SshClient/Renci.SshNet/Channels/Channel.cs +++ b/Renci.SshClient/Renci.SshNet/Channels/Channel.cs @@ -1,4 +1,5 @@ using System; +using System.Linq; using System.Threading; using Renci.SshNet.Common; using Renci.SshNet.Messages; @@ -12,14 +13,16 @@ namespace Renci.SshNet.Channels /// internal abstract class Channel : IDisposable { - private EventWaitHandle _channelClosedWaitHandle = new AutoResetEvent(false); + private EventWaitHandle _channelClosedWaitHandle = new ManualResetEvent(false); - private EventWaitHandle _channelWindowAdjustWaitHandle = new AutoResetEvent(false); + private EventWaitHandle _channelServerWindowAdjustWaitHandle = new ManualResetEvent(false); private EventWaitHandle _errorOccuredWaitHandle = new ManualResetEvent(false); private EventWaitHandle _disconnectedWaitHandle = new ManualResetEvent(false); + private object _serverWindowSizeLock = new object(); + private bool _closeMessageSent = false; private uint _initialWindowSize = 0x100000; @@ -266,7 +269,11 @@ namespace Renci.SshNet.Channels /// The bytes to add. protected virtual void OnWindowAdjust(uint bytesToAdd) { - this.ServerWindowSize += bytesToAdd; + lock (this._serverWindowSizeLock) + { + this.ServerWindowSize += bytesToAdd; + } + this._channelServerWindowAdjustWaitHandle.Set(); } /// @@ -410,20 +417,36 @@ namespace Renci.SshNet.Channels /// Sends channel data message to the servers. /// /// This method takes care of managing the window size. - /// Channel data message. + /// Channel data message. protected void SendMessage(ChannelDataMessage message) { // Send channel messages only while channel is open if (!this.IsOpen) return; - if (this.ServerWindowSize < 1) + var messageLength = message.Data.Length; + do { - // Wait for window to be adjust - this._session.WaitHandle(this._channelWindowAdjustWaitHandle); - } + lock (this._serverWindowSizeLock) + { + var serverWindowSize = this.ServerWindowSize; + if (serverWindowSize < messageLength) + { + // Wait for window to be big enough for this message + this._channelServerWindowAdjustWaitHandle.Reset(); + } + else + { + this.ServerWindowSize -= (uint)messageLength; + break; + } + } + + // Wait for window to change + this.WaitHandle(this._channelServerWindowAdjustWaitHandle); + + } while (true); - this.ServerWindowSize -= (uint)message.Data.Length; this._session.SendMessage(message); } @@ -438,13 +461,29 @@ namespace Renci.SshNet.Channels if (!this.IsOpen) return; - if (this.ServerWindowSize < 1) + var messageLength = message.Data.Length; + do { - // Wait for window to be adjust - this._session.WaitHandle(this._channelWindowAdjustWaitHandle); - } + lock (this._serverWindowSizeLock) + { + var serverWindowSize = this.ServerWindowSize; + if (serverWindowSize < messageLength) + { + // Wait for window to be big enough for this message + this._channelServerWindowAdjustWaitHandle.Reset(); + } + else + { + this.ServerWindowSize -= (uint)messageLength; + break; + } + } + + // Wait for window to change + this.WaitHandle(this._channelServerWindowAdjustWaitHandle); + + } while (true); - this.ServerWindowSize -= (uint)message.Data.Length; this._session.SendMessage(message); } @@ -496,7 +535,6 @@ namespace Renci.SshNet.Channels } } - private void Session_Disconnected(object sender, EventArgs e) { // If objected is disposed or being disposed don't handle this event @@ -546,8 +584,6 @@ namespace Renci.SshNet.Channels if (e.Message.LocalChannelNumber == this.LocalChannelNumber) { this.OnWindowAdjust(e.Message.BytesToAdd); - - this._channelWindowAdjustWaitHandle.Set(); } } @@ -672,10 +708,10 @@ namespace Renci.SshNet.Channels this._channelClosedWaitHandle.Dispose(); this._channelClosedWaitHandle = null; } - if (this._channelWindowAdjustWaitHandle != null) + if (this._channelServerWindowAdjustWaitHandle != null) { - this._channelWindowAdjustWaitHandle.Dispose(); - this._channelWindowAdjustWaitHandle = null; + this._channelServerWindowAdjustWaitHandle.Dispose(); + this._channelServerWindowAdjustWaitHandle = null; } if (this._errorOccuredWaitHandle != null) { diff --git a/Renci.SshClient/Renci.SshNet/Messages/Connection/ChannelExtendedDataMessage.cs b/Renci.SshClient/Renci.SshNet/Messages/Connection/ChannelExtendedDataMessage.cs index a79ba344..e3e692fd 100644 --- a/Renci.SshClient/Renci.SshNet/Messages/Connection/ChannelExtendedDataMessage.cs +++ b/Renci.SshClient/Renci.SshNet/Messages/Connection/ChannelExtendedDataMessage.cs @@ -28,9 +28,11 @@ /// Initializes a new instance of the class. /// /// The local channel number. - public ChannelExtendedDataMessage(uint localChannelNumber) + public ChannelExtendedDataMessage(uint localChannelNumber, uint dataTypeCode, byte[] data) { this.LocalChannelNumber = localChannelNumber; + this.DataTypeCode = dataTypeCode; + this.Data = data; } /// diff --git a/Renci.SshClient/Renci.SshNet/Session.cs b/Renci.SshClient/Renci.SshNet/Session.cs index f6a5e079..a97fd7ad 100644 --- a/Renci.SshClient/Renci.SshNet/Session.cs +++ b/Renci.SshClient/Renci.SshNet/Session.cs @@ -91,7 +91,7 @@ namespace Renci.SshNet /// /// WaitHandle to signal that exception was thrown by another thread. /// - private EventWaitHandle _exceptionWaitHandle = new AutoResetEvent(false); + private EventWaitHandle _exceptionWaitHandle = new ManualResetEvent(false); /// /// WaitHandle to signal that key exchange was completed. @@ -637,11 +637,7 @@ namespace Renci.SshNet switch (EventWaitHandle.WaitAny(waitHandles, this.ConnectionInfo.Timeout)) { case 0: - { - var exception = this._exception; - this._exception = null; - throw exception; - } + throw this._exception; case System.Threading.WaitHandle.WaitTimeout: this.SendDisconnect(DisconnectReason.ByApplication, "Operation timeout"); throw new SshOperationTimeoutException("Session operation has timed out"); diff --git a/Renci.SshClient/Renci.SshNet/Sftp/SftpFile.cs b/Renci.SshClient/Renci.SshNet/Sftp/SftpFile.cs index afe81f35..8f2d45a3 100644 --- a/Renci.SshClient/Renci.SshNet/Sftp/SftpFile.cs +++ b/Renci.SshClient/Renci.SshNet/Sftp/SftpFile.cs @@ -3,6 +3,7 @@ using System.Collections.Generic; using System.Linq; using System.Security; using System.Globalization; +using Renci.SshNet.Common; namespace Renci.SshNet.Sftp { @@ -27,6 +28,9 @@ namespace Renci.SshNet.Sftp /// or is null. internal SftpFile(SftpSession sftpSession, string fullName, SftpFileAttributes attributes) { + if (sftpSession == null) + throw new SshConnectionException("Client not connected."); + if (attributes == null) throw new ArgumentNullException("attributes"); diff --git a/Renci.SshClient/Renci.SshNet/Sftp/SftpFileStream.cs b/Renci.SshClient/Renci.SshNet/Sftp/SftpFileStream.cs index d5891939..ce124e86 100644 --- a/Renci.SshClient/Renci.SshNet/Sftp/SftpFileStream.cs +++ b/Renci.SshClient/Renci.SshNet/Sftp/SftpFileStream.cs @@ -209,6 +209,9 @@ namespace Renci.SshNet.Sftp internal SftpFileStream(SftpSession session, string path, FileMode mode, FileAccess access, int bufferSize, bool useAsync) { // Validate the parameters. + if (session == null) + throw new SshConnectionException("Client not connected."); + if (path == null) { throw new ArgumentNullException("path"); diff --git a/Renci.SshClient/Renci.SshNet/SftpClient.cs b/Renci.SshClient/Renci.SshNet/SftpClient.cs index 8f323c3e..4e58bbb4 100644 --- a/Renci.SshClient/Renci.SshNet/SftpClient.cs +++ b/Renci.SshClient/Renci.SshNet/SftpClient.cs @@ -143,6 +143,9 @@ namespace Renci.SshNet if (path == null) throw new ArgumentNullException("path"); + if (this._sftpSession == null) + throw new SshConnectionException("Client not connected."); + this._sftpSession.ChangeDirectory(path); } @@ -176,6 +179,9 @@ namespace Renci.SshNet if (path.IsNullOrWhiteSpace()) throw new ArgumentException(path); + if (this._sftpSession == null) + throw new SshConnectionException("Client not connected."); + var fullPath = this._sftpSession.GetCanonicalPath(path); this._sftpSession.RequestMkDir(fullPath); @@ -194,6 +200,9 @@ namespace Renci.SshNet if (path.IsNullOrWhiteSpace()) throw new ArgumentException("path"); + if (this._sftpSession == null) + throw new SshConnectionException("Client not connected."); + var fullPath = this._sftpSession.GetCanonicalPath(path); this._sftpSession.RequestRmDir(fullPath); @@ -212,6 +221,9 @@ namespace Renci.SshNet if (path.IsNullOrWhiteSpace()) throw new ArgumentException("path"); + if (this._sftpSession == null) + throw new SshConnectionException("Client not connected."); + var fullPath = this._sftpSession.GetCanonicalPath(path); this._sftpSession.RequestRemove(fullPath); @@ -250,6 +262,9 @@ namespace Renci.SshNet if (newPath == null) throw new ArgumentNullException("newPath"); + if (this._sftpSession == null) + throw new SshConnectionException("Client not connected."); + var oldFullPath = this._sftpSession.GetCanonicalPath(oldPath); var newFullPath = this._sftpSession.GetCanonicalPath(newPath); @@ -281,6 +296,9 @@ namespace Renci.SshNet if (linkPath.IsNullOrWhiteSpace()) throw new ArgumentException("linkPath"); + if (this._sftpSession == null) + throw new SshConnectionException("Client not connected."); + var fullPath = this._sftpSession.GetCanonicalPath(path); var linkFullPath = this._sftpSession.GetCanonicalPath(linkPath); @@ -376,6 +394,9 @@ namespace Renci.SshNet if (path == null) throw new ArgumentNullException("path"); + if (this._sftpSession == null) + throw new SshConnectionException("Client not connected."); + var fullPath = this._sftpSession.GetCanonicalPath(path); var attributes = this._sftpSession.RequestLStat(fullPath); @@ -397,6 +418,9 @@ namespace Renci.SshNet if (path.IsNullOrWhiteSpace()) throw new ArgumentException("path"); + if (this._sftpSession == null) + throw new SshConnectionException("Client not connected."); + var fullPath = this._sftpSession.GetFullRemotePath(path); if (this._sftpSession.RequestRealPath(fullPath, true) == null) @@ -755,6 +779,9 @@ namespace Renci.SshNet if (path == null) throw new ArgumentNullException("path"); + if (this._sftpSession == null) + throw new SshConnectionException("Client not connected."); + var fullPath = this._sftpSession.GetCanonicalPath(path); return this._sftpSession.RequestStatVfs(fullPath); @@ -1285,6 +1312,9 @@ namespace Renci.SshNet /// is null. public SftpFileAttributes GetAttributes(string path) { + if (this._sftpSession == null) + throw new SshConnectionException("Client not connected."); + var fullPath = this._sftpSession.GetCanonicalPath(path); return this._sftpSession.RequestLStat(fullPath); @@ -1298,6 +1328,9 @@ namespace Renci.SshNet /// is null. public void SetAttributes(string path, SftpFileAttributes fileAttributes) { + if (this._sftpSession == null) + throw new SshConnectionException("Client not connected."); + var fullPath = this._sftpSession.GetCanonicalPath(path); this._sftpSession.RequestSetStat(fullPath, fileAttributes); @@ -1328,6 +1361,9 @@ namespace Renci.SshNet if (path == null) throw new ArgumentNullException("path"); + if (this._sftpSession == null) + throw new SshConnectionException("Client not connected."); + var fullPath = this._sftpSession.GetCanonicalPath(path); var handle = this._sftpSession.RequestOpenDir(fullPath); @@ -1380,6 +1416,9 @@ namespace Renci.SshNet if (path.IsNullOrWhiteSpace()) throw new ArgumentException("path"); + if (this._sftpSession == null) + throw new SshConnectionException("Client not connected."); + var fullPath = this._sftpSession.GetCanonicalPath(path); var handle = this._sftpSession.RequestOpen(fullPath, Flags.Read); @@ -1434,6 +1473,9 @@ namespace Renci.SshNet if (path.IsNullOrWhiteSpace()) throw new ArgumentException("path"); + if (this._sftpSession == null) + throw new SshConnectionException("Client not connected."); + var fullPath = this._sftpSession.GetCanonicalPath(path); var handle = this._sftpSession.RequestOpen(fullPath, flags); @@ -1444,7 +1486,7 @@ namespace Renci.SshNet var bytesRead = input.Read(buffer, 0, buffer.Length); var expectedResponses = 0; - var expectedResponsesLock = new object(); + var responseReceivedWaitHandle = new AutoResetEvent(false); do { @@ -1467,25 +1509,18 @@ namespace Renci.SshNet { if (s.StatusCode == StatusCodes.Ok) { - lock (expectedResponsesLock) + expectedResponses--; + responseReceivedWaitHandle.Set(); + + // Call callback to report number of bytes written + if (uploadCallback != null) { - expectedResponses--; - - // Call callback to report number of bytes written - if (uploadCallback != null) - { - // Execute callback on different thread - this.ExecuteThread(() => { uploadCallback(writtenBytes); }); - } - - Monitor.Pulse(expectedResponsesLock); + // Execute callback on different thread + this.ExecuteThread(() => { uploadCallback(writtenBytes); }); } } }); - lock (expectedResponsesLock) - { - expectedResponses++; - } + expectedResponses++; offset += (uint)bytesRead; @@ -1494,10 +1529,7 @@ namespace Renci.SshNet else if (expectedResponses > 0) { // Wait for expectedResponses to change - lock (expectedResponsesLock) - { - Monitor.Wait(expectedResponsesLock); - } + this._sftpSession.WaitHandle(responseReceivedWaitHandle, this.OperationTimeout); } } while (expectedResponses > 0 || bytesRead > 0); diff --git a/Renci.SshClient/Renci.SshNet/SshCommand.cs b/Renci.SshClient/Renci.SshNet/SshCommand.cs index f43a8c12..8edba04f 100644 --- a/Renci.SshClient/Renci.SshNet/SshCommand.cs +++ b/Renci.SshClient/Renci.SshNet/SshCommand.cs @@ -484,11 +484,7 @@ namespace Renci.SshNet switch (EventWaitHandle.WaitAny(waitHandles, this.CommandTimeout)) { case 0: - { - var exception = this._exception; - this._exception = null; - throw exception; - } + throw this._exception; case System.Threading.WaitHandle.WaitTimeout: throw new SshOperationTimeoutException(string.Format(CultureInfo.CurrentCulture, "Command '{0}' has timed out.", this.CommandText)); default: diff --git a/Renci.SshClient/Renci.SshNet/SubsystemSession.cs b/Renci.SshClient/Renci.SshNet/SubsystemSession.cs index b15a8a73..4ba1651f 100644 --- a/Renci.SshClient/Renci.SshNet/SubsystemSession.cs +++ b/Renci.SshClient/Renci.SshNet/SubsystemSession.cs @@ -27,9 +27,9 @@ namespace Renci.SshNet.Sftp private Exception _exception; - private EventWaitHandle _errorOccuredWaitHandle = new AutoResetEvent(false); + private EventWaitHandle _errorOccuredWaitHandle = new ManualResetEvent(false); - private EventWaitHandle _channelClosedWaitHandle = new AutoResetEvent(false); + private EventWaitHandle _channelClosedWaitHandle = new ManualResetEvent(false); /// /// Specifies a timeout to wait for operation to complete @@ -165,11 +165,7 @@ namespace Renci.SshNet.Sftp switch (EventWaitHandle.WaitAny(waitHandles, operationTimeout)) { case 0: - { - var exception = this._exception; - this._exception = null; - throw exception; - } + throw this._exception; case 1: throw new SshException("Channel was closed."); case System.Threading.WaitHandle.WaitTimeout: @@ -197,7 +193,7 @@ namespace Renci.SshNet.Sftp #region IDisposable Members private bool _isDisposed = false; - + /// /// Performs application-defined tasks associated with freeing, releasing, or resetting unmanaged resources. ///