Skip to content

Commit

Permalink
Fix sftp async methods not observing error conditions (#1510)
Browse files Browse the repository at this point in the history
* Fix sftp async methods not observing error conditions

* Update ISftpClient
  • Loading branch information
Rob-Hague authored Oct 8, 2024
1 parent 1a8839e commit fbedaab
Show file tree
Hide file tree
Showing 7 changed files with 570 additions and 343 deletions.
1 change: 0 additions & 1 deletion src/Renci.SshNet/ISftpClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ public interface ISftpClient : IBaseClient
/// The timeout to wait until an operation completes. The default value is negative
/// one (-1) milliseconds, which indicates an infinite timeout period.
/// </value>
/// <exception cref="ObjectDisposedException">The method was called after the client was disposed.</exception>
/// <exception cref="ArgumentOutOfRangeException"><paramref name="value"/> represents a value that is less than -1 or greater than <see cref="int.MaxValue"/> milliseconds.</exception>
TimeSpan OperationTimeout { get; set; }

Expand Down
6 changes: 3 additions & 3 deletions src/Renci.SshNet/ISubsystemSession.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@ namespace Renci.SshNet
internal interface ISubsystemSession : IDisposable
{
/// <summary>
/// Gets or set the number of seconds to wait for an operation to complete.
/// Gets or sets the number of milliseconds to wait for an operation to complete.
/// </summary>
/// <value>
/// The number of seconds to wait for an operation to complete, or <c>-1</c> to wait indefinitely.
/// The number of milliseconds to wait for an operation to complete, or <c>-1</c> to wait indefinitely.
/// </value>
int OperationTimeout { get; }
int OperationTimeout { get; set; }

/// <summary>
/// Gets a value indicating whether this session is open.
Expand Down
533 changes: 238 additions & 295 deletions src/Renci.SshNet/Sftp/SftpSession.cs

Large diffs are not rendered by default.

10 changes: 5 additions & 5 deletions src/Renci.SshNet/SftpClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,21 +46,21 @@ public class SftpClient : BaseClient, ISftpClient
/// The timeout to wait until an operation completes. The default value is negative
/// one (-1) milliseconds, which indicates an infinite timeout period.
/// </value>
/// <exception cref="ObjectDisposedException">The method was called after the client was disposed.</exception>
/// <exception cref="ArgumentOutOfRangeException"><paramref name="value"/> represents a value that is less than -1 or greater than <see cref="int.MaxValue"/> milliseconds.</exception>
public TimeSpan OperationTimeout
{
get
{
CheckDisposed();

return TimeSpan.FromMilliseconds(_operationTimeout);
}
set
{
CheckDisposed();

_operationTimeout = value.AsTimeout(nameof(OperationTimeout));

if (_sftpSession is { } sftpSession)
{
sftpSession.OperationTimeout = _operationTimeout;
}
}
}

Expand Down
63 changes: 56 additions & 7 deletions src/Renci.SshNet/SubsystemSession.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
using System.Globalization;
using System.Runtime.ExceptionServices;
using System.Threading;
using System.Threading.Tasks;

using Renci.SshNet.Abstractions;
using Renci.SshNet.Channels;
Expand Down Expand Up @@ -29,13 +30,8 @@ internal abstract class SubsystemSession : ISubsystemSession
private EventWaitHandle _channelClosedWaitHandle = new ManualResetEvent(initialState: false);
private bool _isDisposed;

/// <summary>
/// Gets or set the number of seconds to wait for an operation to complete.
/// </summary>
/// <value>
/// The number of seconds to wait for an operation to complete, or -1 to wait indefinitely.
/// </value>
public int OperationTimeout { get; private set; }
/// <inheritdoc/>
public int OperationTimeout { get; set; }

/// <summary>
/// Occurs when an error occurred.
Expand Down Expand Up @@ -250,6 +246,59 @@ public void WaitOnHandle(WaitHandle waitHandle, int millisecondsTimeout)
}
}

protected async Task<T> WaitOnHandleAsync<T>(TaskCompletionSource<T> tcs, int millisecondsTimeout, CancellationToken cancellationToken)
{
cancellationToken.ThrowIfCancellationRequested();

var errorOccuredReg = ThreadPool.RegisterWaitForSingleObject(
_errorOccuredWaitHandle,
(tcs, _) => ((TaskCompletionSource<T>)tcs).TrySetException(_exception),
state: tcs,
millisecondsTimeOutInterval: -1,
executeOnlyOnce: true);

var sessionDisconnectedReg = ThreadPool.RegisterWaitForSingleObject(
_sessionDisconnectedWaitHandle,
static (tcs, _) => ((TaskCompletionSource<T>)tcs).TrySetException(new SshException("Connection was closed by the server.")),
state: tcs,
millisecondsTimeOutInterval: -1,
executeOnlyOnce: true);

var channelClosedReg = ThreadPool.RegisterWaitForSingleObject(
_channelClosedWaitHandle,
static (tcs, _) => ((TaskCompletionSource<T>)tcs).TrySetException(new SshException("Channel was closed.")),
state: tcs,
millisecondsTimeOutInterval: -1,
executeOnlyOnce: true);

using var timeoutCts = new CancellationTokenSource(millisecondsTimeout);
using var linkedCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, timeoutCts.Token);

using var tokenReg = linkedCts.Token.Register(
static s =>
{
(var tcs, var cancellationToken) = ((TaskCompletionSource<T>, CancellationToken))s;
_ = tcs.TrySetCanceled(cancellationToken);
},
state: (tcs, cancellationToken),
useSynchronizationContext: false);

try
{
return await tcs.Task.ConfigureAwait(false);
}
catch (OperationCanceledException oce) when (timeoutCts.IsCancellationRequested)
{
throw new SshOperationTimeoutException("Operation has timed out.", oce);
}
finally
{
_ = errorOccuredReg.Unregister(waitObject: null);
_ = sessionDisconnectedReg.Unregister(waitObject: null);
_ = channelClosedReg.Unregister(waitObject: null);
}
}

/// <summary>
/// Blocks the current thread until the specified <see cref="WaitHandle"/> gets signaled, using a
/// 32-bit signed integer to specify the time interval in milliseconds.
Expand Down
32 changes: 0 additions & 32 deletions test/Renci.SshNet.Tests/Classes/SftpClientTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -115,37 +115,5 @@ public void OperationTimeout_GreaterThanLowerLimit()
Assert.AreEqual("OperationTimeout", ex.ParamName);
}
}

[TestMethod]
public void OperationTimeout_Disposed()
{
var connectionInfo = new PasswordConnectionInfo("host", 22, "admin", "pwd");
var target = new SftpClient(connectionInfo);
target.Dispose();

// getter
try
{
var actual = target.OperationTimeout;
Assert.Fail("Should have failed, but returned: " + actual);
}
catch (ObjectDisposedException ex)
{
Assert.IsNull(ex.InnerException);
Assert.AreEqual(typeof(SftpClient).FullName, ex.ObjectName);
}

// setter
try
{
target.OperationTimeout = TimeSpan.FromMilliseconds(5);
Assert.Fail();
}
catch (ObjectDisposedException ex)
{
Assert.IsNull(ex.InnerException);
Assert.AreEqual(typeof(SftpClient).FullName, ex.ObjectName);
}
}
}
}
Loading

0 comments on commit fbedaab

Please sign in to comment.