Fix deadlock in SftpClient.UploadFile upon error

src/Renci.SshNet/Sftp/SftpSession.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Globalization;
 using System.Text;
 using System.Threading;
@@ -914,6 +915,8 @@ public void RequestWrite(byte[] handle,
                                  AutoResetEvent wait,
                                  Action<SftpStatusResponse> writeCompleted = null)
         {
+            Debug.Assert((wait is null) != (writeCompleted is null), "Should have one parameter or the other.");
+
             SshException exception = null;
 
             var request = new SftpWriteRequest(ProtocolVersion,
@@ -925,22 +928,27 @@ public void RequestWrite(byte[] handle,
                                                length,
                                                response =>
                                                {
-                                                   writeCompleted?.Invoke(response);
-
-                                                   exception = GetSftpException(response);
-                                                   wait?.SetIgnoringObjectDisposed();
+                                                   if (writeCompleted is not null)
+                                                   {
+                                                       writeCompleted.Invoke(response);
+                                                   }
+                                                   else
+                                                   {
+                                                       exception = GetSftpException(response);
+                                                       wait.SetIgnoringObjectDisposed();
+                                                   }
                                                });
 
             SendRequest(request);
 
             if (wait is not null)
             {
                 WaitOnHandle(wait, OperationTimeout);
-            }
 
-            if (exception is not null)
-            {
-                throw exception;
+                if (exception is not null)
+                {
+                    throw exception;
+                }
             }
         }
 
@@ -2272,7 +2280,7 @@ public uint CalculateOptimalWriteLength(uint bufferSize, byte[] handle)
             return Math.Min(bufferSize, maximumPacketSize) - lengthOfNonDataProtocolFields;
         }
 
-        private static SshException GetSftpException(SftpStatusResponse response)
+        internal static SshException GetSftpException(SftpStatusResponse response)
         {
 #pragma warning disable IDE0010 // Add missing cases
             switch (response.StatusCode)

src/Renci.SshNet/SftpClient.cs

@@ -1,11 +1,13 @@
 #nullable enable
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Diagnostics.CodeAnalysis;
 using System.Globalization;
 using System.IO;
 using System.Net;
 using System.Runtime.CompilerServices;
+using System.Runtime.ExceptionServices;
 using System.Text;
 using System.Threading;
 using System.Threading.Tasks;
@@ -2456,56 +2458,79 @@ private void InternalUploadFile(Stream input, string path, Flags flags, SftpUplo
             // create buffer of optimal length
             var buffer = new byte[_sftpSession.CalculateOptimalWriteLength(_bufferSize, handle)];
 
-            var bytesRead = input.Read(buffer, 0, buffer.Length);
+            int bytesRead;
             var expectedResponses = 0;
-            var responseReceivedWaitHandle = new AutoResetEvent(initialState: false);
+            using var mres = new ManualResetEventSlim(initialState: false);
 
-            do
+            ExceptionDispatchInfo? exception = null;
+
+            while ((bytesRead = input.Read(buffer, 0, buffer.Length)) != 0)
             {
-                // Cancel upload
                 if (asyncResult is not null && asyncResult.IsUploadCanceled)
                 {
                     break;
                 }
 
-                if (bytesRead > 0)
+                exception?.Throw();
+
+                var writtenBytes = offset + (ulong)bytesRead;
+
+                _ = Interlocked.Increment(ref expectedResponses);
+                mres.Reset();
+
+                _sftpSession.RequestWrite(handle, offset, buffer, offset: 0, bytesRead, wait: null, s =>
                 {
-                    var writtenBytes = offset + (ulong)bytesRead;
+                    var setHandle = false;
 
-                    _sftpSession.RequestWrite(handle, offset, buffer, offset: 0, bytesRead, wait: null, s =>
+                    try
+                    {
+                        if (Interlocked.Decrement(ref expectedResponses) == 0)
                         {
-                            if (s.StatusCode == StatusCodes.Ok)
-                            {
-                                _ = Interlocked.Decrement(ref expectedResponses);
-                                _ = responseReceivedWaitHandle.Set();
+                            setHandle = true;
+                        }
 
-                                asyncResult?.Update(writtenBytes);
+                        if (Sftp.SftpSession.GetSftpException(s) is Exception ex)
+                        {
+                            exception = ExceptionDispatchInfo.Capture(ex);
+                        }
 
-                                // Call callback to report number of bytes written
-                                if (uploadCallback is not null)
-                                {
-                                    // Execute callback on different thread
-                                    ThreadAbstraction.ExecuteThread(() => uploadCallback(writtenBytes));
-                                }
-                            }
-                        });
+                        if (exception is not null)
+                        {
+                            setHandle = true;
+                            return;
+                        }
 
-                    _ = Interlocked.Increment(ref expectedResponses);
+                        Debug.Assert(s.StatusCode == StatusCodes.Ok);
 
-                    offset += (ulong)bytesRead;
+                        asyncResult?.Update(writtenBytes);
 
-                    bytesRead = input.Read(buffer, 0, buffer.Length);
-                }
-                else if (expectedResponses > 0)
-                {
-                    // Wait for expectedResponses to change
-                    _sftpSession.WaitOnHandle(responseReceivedWaitHandle, _operationTimeout);
-                }
+                        // Call callback to report number of bytes written
+                        if (uploadCallback is not null)
+                        {
+                            // Execute callback on different thread
+                            ThreadAbstraction.ExecuteThread(() => uploadCallback(writtenBytes));
+                        }
+                    }
+                    finally
+                    {
+                        if (setHandle)
+                        {
+                            mres.Set();
+                        }
+                    }
+                });
+
+                offset += (ulong)bytesRead;
             }
-            while (expectedResponses > 0 || bytesRead > 0);
+
+            if (expectedResponses != 0)
+            {
+                _sftpSession.WaitOnHandle(mres.WaitHandle, _operationTimeout);
+            }
+
+            exception?.Throw();
 
             _sftpSession.RequestClose(handle);
-            responseReceivedWaitHandle.Dispose();
         }
 
         private async Task InternalUploadFileAsync(Stream input, string path, CancellationToken cancellationToken)