Skip to content

Commit 3422faa

Browse files
committed
Fix deadlock in SftpClient.UploadFile upon error
1 parent d08c4aa commit 3422faa

2 files changed

Lines changed: 57 additions & 32 deletions

File tree

src/Renci.SshNet/Sftp/SftpSession.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2272,7 +2272,7 @@ public uint CalculateOptimalWriteLength(uint bufferSize, byte[] handle)
22722272
return Math.Min(bufferSize, maximumPacketSize) - lengthOfNonDataProtocolFields;
22732273
}
22742274

2275-
private static SshException GetSftpException(SftpStatusResponse response)
2275+
internal static SshException GetSftpException(SftpStatusResponse response)
22762276
{
22772277
#pragma warning disable IDE0010 // Add missing cases
22782278
switch (response.StatusCode)

src/Renci.SshNet/SftpClient.cs

Lines changed: 56 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
#nullable enable
22
using System;
33
using System.Collections.Generic;
4+
using System.Diagnostics;
45
using System.Diagnostics.CodeAnalysis;
56
using System.Globalization;
67
using System.IO;
78
using System.Net;
89
using System.Runtime.CompilerServices;
10+
using System.Runtime.ExceptionServices;
911
using System.Text;
1012
using System.Threading;
1113
using System.Threading.Tasks;
@@ -2456,56 +2458,79 @@ private void InternalUploadFile(Stream input, string path, Flags flags, SftpUplo
24562458
// create buffer of optimal length
24572459
var buffer = new byte[_sftpSession.CalculateOptimalWriteLength(_bufferSize, handle)];
24582460

2459-
var bytesRead = input.Read(buffer, 0, buffer.Length);
2461+
int bytesRead;
24602462
var expectedResponses = 0;
2461-
var responseReceivedWaitHandle = new AutoResetEvent(initialState: false);
2463+
using var mres = new ManualResetEventSlim(initialState: false);
24622464

2463-
do
2465+
ExceptionDispatchInfo? exception = null;
2466+
2467+
while ((bytesRead = input.Read(buffer, 0, buffer.Length)) != 0)
24642468
{
2465-
// Cancel upload
24662469
if (asyncResult is not null && asyncResult.IsUploadCanceled)
24672470
{
24682471
break;
24692472
}
24702473

2471-
if (bytesRead > 0)
2474+
exception?.Throw();
2475+
2476+
var writtenBytes = offset + (ulong)bytesRead;
2477+
2478+
_ = Interlocked.Increment(ref expectedResponses);
2479+
mres.Reset();
2480+
2481+
_sftpSession.RequestWrite(handle, offset, buffer, offset: 0, bytesRead, wait: null, s =>
24722482
{
2473-
var writtenBytes = offset + (ulong)bytesRead;
2483+
var setHandle = false;
24742484

2475-
_sftpSession.RequestWrite(handle, offset, buffer, offset: 0, bytesRead, wait: null, s =>
2485+
try
2486+
{
2487+
if (Interlocked.Decrement(ref expectedResponses) == 0)
24762488
{
2477-
if (s.StatusCode == StatusCodes.Ok)
2478-
{
2479-
_ = Interlocked.Decrement(ref expectedResponses);
2480-
_ = responseReceivedWaitHandle.Set();
2489+
setHandle = true;
2490+
}
24812491

2482-
asyncResult?.Update(writtenBytes);
2492+
if (Sftp.SftpSession.GetSftpException(s) is Exception ex)
2493+
{
2494+
exception = ExceptionDispatchInfo.Capture(ex);
2495+
}
24832496

2484-
// Call callback to report number of bytes written
2485-
if (uploadCallback is not null)
2486-
{
2487-
// Execute callback on different thread
2488-
ThreadAbstraction.ExecuteThread(() => uploadCallback(writtenBytes));
2489-
}
2490-
}
2491-
});
2497+
if (exception is not null)
2498+
{
2499+
setHandle = true;
2500+
return;
2501+
}
24922502

2493-
_ = Interlocked.Increment(ref expectedResponses);
2503+
Debug.Assert(s.StatusCode == StatusCodes.Ok);
24942504

2495-
offset += (ulong)bytesRead;
2505+
asyncResult?.Update(writtenBytes);
24962506

2497-
bytesRead = input.Read(buffer, 0, buffer.Length);
2498-
}
2499-
else if (expectedResponses > 0)
2500-
{
2501-
// Wait for expectedResponses to change
2502-
_sftpSession.WaitOnHandle(responseReceivedWaitHandle, _operationTimeout);
2503-
}
2507+
// Call callback to report number of bytes written
2508+
if (uploadCallback is not null)
2509+
{
2510+
// Execute callback on different thread
2511+
ThreadAbstraction.ExecuteThread(() => uploadCallback(writtenBytes));
2512+
}
2513+
}
2514+
finally
2515+
{
2516+
if (setHandle)
2517+
{
2518+
mres.Set();
2519+
}
2520+
}
2521+
});
2522+
2523+
offset += (ulong)bytesRead;
25042524
}
2505-
while (expectedResponses > 0 || bytesRead > 0);
2525+
2526+
if (expectedResponses != 0)
2527+
{
2528+
_sftpSession.WaitOnHandle(mres.WaitHandle, _operationTimeout);
2529+
}
2530+
2531+
exception?.Throw();
25062532

25072533
_sftpSession.RequestClose(handle);
2508-
responseReceivedWaitHandle.Dispose();
25092534
}
25102535

25112536
private async Task InternalUploadFileAsync(Stream input, string path, CancellationToken cancellationToken)

0 commit comments

Comments
 (0)