From 7df60bb01cbd9b376eb0ef91b982967b355342cb Mon Sep 17 00:00:00 2001
From: Rob Hague <rob.hague00@gmail.com>
Date: Mon, 4 May 2026 22:53:14 +0200
Subject: [PATCH] Tweak internal IProgress usage for APM and sync
 UploadFile/DownloadFile

Changes to support IProgress<> callback on UploadAsync/DownloadAsync meant wrapping
the Action<> callback on existing methods in a Progress<>, which posts the callback
onto the current synchronisation context rather than the threadpool. For the legacy
APM methods (Begin[..]), let's just preserve their old behaviour.

For the synchronous methods, posting to the synchronisation context is probably the
worst choice (because if there is one, the method itself is running there). We can
either revert to the threadpool as well, or take the opportunity to invoke the
callback synchronously, which is a behavioural change but probably the least
surprising behaviour for a synchronous method.
---
 src/Renci.SshNet/SftpClient.cs | 85 +++++++++++++++++++++++++---------
 1 file changed, 62 insertions(+), 23 deletions(-)

diff --git a/src/Renci.SshNet/SftpClient.cs b/src/Renci.SshNet/SftpClient.cs
index 960c6261f..2837153e2 100644
--- a/src/Renci.SshNet/SftpClient.cs
+++ b/src/Renci.SshNet/SftpClient.cs
@@ -905,7 +905,7 @@ public void DownloadFile(string path, Stream output, Action<ulong>? downloadCall
 
             if (downloadCallback != null)
             {
-                downloadProgress = new Progress<DownloadFileProgressReport>(r => downloadCallback(r.TotalBytesDownloaded));
+                downloadProgress = new SynchronousProgress<DownloadFileProgressReport>(r => downloadCallback(r.TotalBytesDownloaded));
             }
 
             InternalDownloadFile(
@@ -934,7 +934,7 @@ public Task DownloadFileAsync(string path, Stream output, IProgress<DownloadFile
                 path,
                 output,
                 asyncResult: null,
-                downloadProgress: downloadProgress,
+                downloadProgress,
                 isAsync: true,
                 cancellationToken);
         }
@@ -1011,7 +1011,11 @@ public IAsyncResult BeginDownloadFile(string path, Stream output, AsyncCallback?
 
             if (downloadCallback != null)
             {
-                downloadProgress = new Progress<DownloadFileProgressReport>(r => downloadCallback(r.TotalBytesDownloaded));
+                // The System.Progress<T> ctor captures the current synchronization context
+                // and posts the progress reports to it. For back-compat with previous
+                // versions which always posted the callback to the threadpool regardless of
+                // sync context, we use a custom IProgress<T> impl.
+                downloadProgress = new ThreadPoolProgress<DownloadFileProgressReport>(r => downloadCallback(r.TotalBytesDownloaded));
             }
 
             var asyncResult = new SftpDownloadAsyncResult(asyncCallback, state);
@@ -1089,7 +1093,7 @@ public void UploadFile(Stream input, string path, bool canOverride, Action<ulong
 
             if (uploadCallback != null)
             {
-                uploadProgress = new Progress<UploadFileProgressReport>(r => uploadCallback(r.TotalBytesUploaded));
+                uploadProgress = new SynchronousProgress<UploadFileProgressReport>(r => uploadCallback(r.TotalBytesUploaded));
             }
 
             InternalUploadFile(
@@ -1273,7 +1277,11 @@ public IAsyncResult BeginUploadFile(Stream input, string path, bool canOverride,
 
             if (uploadCallback != null)
             {
-                uploadProgress = new Progress<UploadFileProgressReport>(r => uploadCallback(r.TotalBytesUploaded));
+                // The System.Progress<T> ctor captures the current synchronization context
+                // and posts the progress reports to it. For back-compat with previous
+                // versions which always posted the callback to the threadpool regardless of
+                // sync context, we use a custom IProgress<T> impl.
+                uploadProgress = new ThreadPoolProgress<UploadFileProgressReport>(r => uploadCallback(r.TotalBytesUploaded));
             }
 
             var asyncResult = new SftpUploadAsyncResult(asyncCallback, state);
@@ -2417,16 +2425,10 @@ private async Task InternalDownloadFile(
 
                     asyncResult?.Update(totalBytesRead);
 
-                    if (downloadProgress is not null)
+                    downloadProgress?.Report(new DownloadFileProgressReport()
                     {
-                        // Copy offset to ensure it's not modified between now and execution of callback
-                        var report = new DownloadFileProgressReport()
-                        {
-                            TotalBytesDownloaded = totalBytesRead,
-                        };
-
-                        downloadProgress.Report(report);
-                    }
+                        TotalBytesDownloaded = totalBytesRead
+                    });
                 }
             }
             finally
@@ -2536,16 +2538,10 @@ private async Task InternalUploadFile(
 
                         asyncResult?.Update(writtenBytes);
 
-                        // Call callback to report number of bytes written
-                        if (uploadProgress is not null)
+                        uploadProgress?.Report(new UploadFileProgressReport()
                         {
-                            UploadFileProgressReport report = new()
-                            {
-                                TotalBytesUploaded = writtenBytes,
-                            };
-
-                            uploadProgress.Report(report);
-                        }
+                            TotalBytesUploaded = writtenBytes
+                        });
                     }
                     finally
                     {
@@ -2652,5 +2648,48 @@ private ISftpSession CreateAndConnectToSftpSession()
                 throw;
             }
         }
+
+        /// <summary>
+        /// An <see cref="IProgress{T}"/> implementation that posts callbacks to the threadpool.
+        /// </summary>
+        private sealed class ThreadPoolProgress<T> : IProgress<T>
+        {
+            private readonly Action<T> _handler;
+
+            public ThreadPoolProgress(Action<T> handler)
+            {
+                Debug.Assert(handler != null);
+                _handler = handler!;
+            }
+
+            void IProgress<T>.Report(T value)
+            {
+                _ = ThreadPool.QueueUserWorkItem(static state =>
+                {
+                    var (handler, value) = ((Action<T>, T))state!;
+                    handler(value);
+                },
+                (_handler, value));
+            }
+        }
+
+        /// <summary>
+        /// An <see cref="IProgress{T}"/> implementation that invokes callbacks synchronously.
+        /// </summary>
+        private sealed class SynchronousProgress<T> : IProgress<T>
+        {
+            private readonly Action<T> _handler;
+
+            public SynchronousProgress(Action<T> handler)
+            {
+                Debug.Assert(handler != null);
+                _handler = handler!;
+            }
+
+            void IProgress<T>.Report(T value)
+            {
+                _handler.Invoke(value);
+            }
+        }
     }
 }