Skip to content

Fix sftp async methods not observing error conditions #1510

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

Merged
merged 2 commits into from
Oct 8, 2024
Merged
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: 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