From 936bb42533bbcb4dbefc36c25b95b7d1977d5dd3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Miroslav=20Pokorn=C3=BD?= <m.pokorny@quadient.com>
Date: Thu, 21 May 2020 15:51:29 +0200
Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9B=20AsyncResult=20contains=20invalid?=
 =?UTF-8?q?=20value?=
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- AsyncResult should contain invalid value immediately after async operation is marked as completed
- there was race condition problems with callback method which is invoked on different thread so updating of value is done without any synchronization. So in some cases async operation is marked as completed but async result value is not yet updated and contains invalid value
---
 src/Renci.SshNet/SftpClient.cs | 48 ++++++++++++----------------------
 1 file changed, 16 insertions(+), 32 deletions(-)

diff --git a/src/Renci.SshNet/SftpClient.cs b/src/Renci.SshNet/SftpClient.cs
index 5a0993009..a82e30105 100644
--- a/src/Renci.SshNet/SftpClient.cs
+++ b/src/Renci.SshNet/SftpClient.cs
@@ -473,7 +473,7 @@ public IEnumerable<SftpFile> ListDirectory(string path, Action<int> listCallback
         {
             CheckDisposed();
 
-            return InternalListDirectory(path, listCallback);
+            return InternalListDirectory(path, null, listCallback);
         }
 
         /// <summary>
@@ -497,15 +497,7 @@ public IAsyncResult BeginListDirectory(string path, AsyncCallback asyncCallback,
             {
                 try
                 {
-                    var result = InternalListDirectory(path, count =>
-                    {
-                        asyncResult.Update(count);
-
-                        if (listCallback != null)
-                        {
-                            listCallback(count);
-                        }
-                    });
+                    var result = InternalListDirectory(path, asyncResult, listCallback);
 
                     asyncResult.SetAsCompleted(result, false);
                 }
@@ -718,15 +710,7 @@ public IAsyncResult BeginDownloadFile(string path, Stream output, AsyncCallback
             {
                 try
                 {
-                    InternalDownloadFile(path, output, asyncResult, offset =>
-                    {
-                        asyncResult.Update(offset);
-
-                        if (downloadCallback != null)
-                        {
-                            downloadCallback(offset);
-                        }
-                    });
+                    InternalDownloadFile(path, output, asyncResult, downloadCallback);
 
                     asyncResult.SetAsCompleted(null, false);
                 }
@@ -942,16 +926,7 @@ public IAsyncResult BeginUploadFile(Stream input, string path, bool canOverride,
             {
                 try
                 {
-                    InternalUploadFile(input, path, flags, asyncResult, offset =>
-                    {
-                        asyncResult.Update(offset);
-
-                        if (uploadCallback != null)
-                        {
-                            uploadCallback(offset);
-                        }
-
-                    });
+                    InternalUploadFile(input, path, flags, asyncResult, uploadCallback);
 
                     asyncResult.SetAsCompleted(null, false);
                 }
@@ -1919,7 +1894,7 @@ private IEnumerable<FileInfo> InternalSynchronizeDirectories(string sourcePath,
 
             #region Existing Files at The Destination
 
-            var destFiles = InternalListDirectory(destinationPath, null);
+            var destFiles = InternalListDirectory(destinationPath, null, null);
             var destDict = new Dictionary<string, SftpFile>();
             foreach (var destFile in destFiles)
             {
@@ -1980,13 +1955,14 @@ private IEnumerable<FileInfo> InternalSynchronizeDirectories(string sourcePath,
         /// Internals the list directory.
         /// </summary>
         /// <param name="path">The path.</param>
+        /// <param name="asyncResult">An <see cref="IAsyncResult"/> that references the asynchronous request.</param>
         /// <param name="listCallback">The list callback.</param>
         /// <returns>
         /// A list of files in the specfied directory.
         /// </returns>
         /// <exception cref="ArgumentNullException"><paramref name="path" /> is <b>null</b>.</exception>
         /// <exception cref="SshConnectionException">Client not connected.</exception>
-        private IEnumerable<SftpFile> InternalListDirectory(string path, Action<int> listCallback)
+        private IEnumerable<SftpFile> InternalListDirectory(string path, SftpListDirectoryAsyncResult asyncResult, Action<int> listCallback)
         {
             if (path == null)
                 throw new ArgumentNullException("path");
@@ -2012,6 +1988,9 @@ private IEnumerable<SftpFile> InternalListDirectory(string path, Action<int> lis
                 result.AddRange(from f in files
                                 select new SftpFile(_sftpSession, string.Format(CultureInfo.InvariantCulture, "{0}{1}", basePath, f.Key), f.Value));
 
+                if (asyncResult != null)
+                    asyncResult.Update(result.Count);
+
                 //  Call callback to report number of files read
                 if (listCallback != null)
                 {
@@ -2068,6 +2047,9 @@ private void InternalDownloadFile(string path, Stream output, SftpDownloadAsyncR
 
                     totalBytesRead += (ulong) data.Length;
 
+                    if (asyncResult != null)
+                        asyncResult.Update(totalBytesRead);
+
                     if (downloadCallback != null)
                     {
                         // copy offset to ensure it's not modified between now and execution of callback
@@ -2124,7 +2106,6 @@ private void InternalUploadFile(Stream input, string path, Flags flags, SftpUplo
                 if (bytesRead > 0)
                 {
                     var writtenBytes = offset + (ulong) bytesRead;
-
                     _sftpSession.RequestWrite(handle, offset, buffer, 0, bytesRead, null, s =>
                         {
                             if (s.StatusCode == StatusCodes.Ok)
@@ -2132,6 +2113,9 @@ private void InternalUploadFile(Stream input, string path, Flags flags, SftpUplo
                                 Interlocked.Decrement(ref expectedResponses);
                                 responseReceivedWaitHandle.Set();
 
+                                if (asyncResult != null)
+                                    asyncResult.Update(writtenBytes);
+
                                 //  Call callback to report number of bytes written
                                 if (uploadCallback != null)
                                 {