Skip to content

Commit

Permalink
🐛 AsyncResult contains invalid value
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
miroslavpokorny authored and Rob-Hague committed Sep 24, 2023
1 parent aade354 commit d7688de
Showing 1 changed file with 13 additions and 20 deletions.
33 changes: 13 additions & 20 deletions src/Renci.SshNet/SftpClient.cs
Original file line number Diff line number Diff line change
Expand Up @@ -584,7 +584,7 @@ public IEnumerable<ISftpFile> ListDirectory(string path, Action<int> listCallbac
{
CheckDisposed();

return InternalListDirectory(path, listCallback);
return InternalListDirectory(path, asyncResult: null, listCallback);
}

#if FEATURE_ASYNC_ENUMERABLE
Expand Down Expand Up @@ -669,12 +669,7 @@ public IAsyncResult BeginListDirectory(string path, AsyncCallback asyncCallback,
{
try
{
var result = InternalListDirectory(path, count =>
{
asyncResult.Update(count);
listCallback?.Invoke(count);
});
var result = InternalListDirectory(path, asyncResult, listCallback);
asyncResult.SetAsCompleted(result, completedSynchronously: false);
}
Expand Down Expand Up @@ -899,12 +894,7 @@ public IAsyncResult BeginDownloadFile(string path, Stream output, AsyncCallback
{
try
{
InternalDownloadFile(path, output, asyncResult, offset =>
{
asyncResult.Update(offset);
downloadCallback?.Invoke(offset);
});
InternalDownloadFile(path, output, asyncResult, downloadCallback);
asyncResult.SetAsCompleted(exception: null, completedSynchronously: false);
}
Expand Down Expand Up @@ -1132,11 +1122,7 @@ public IAsyncResult BeginUploadFile(Stream input, string path, bool canOverride,
{
try
{
InternalUploadFile(input, path, flags, asyncResult, offset =>
{
asyncResult.Update(offset);
uploadCallback?.Invoke(offset);
});
InternalUploadFile(input, path, flags, asyncResult, uploadCallback);
asyncResult.SetAsCompleted(exception: null, completedSynchronously: false);
}
Expand Down Expand Up @@ -2200,7 +2186,7 @@ private IEnumerable<FileInfo> InternalSynchronizeDirectories(string sourcePath,

#region Existing Files at The Destination

var destFiles = InternalListDirectory(destinationPath, listCallback: null);
var destFiles = InternalListDirectory(destinationPath, asyncResult: null, listCallback: null);
var destDict = new Dictionary<string, ISftpFile>();
foreach (var destFile in destFiles)
{
Expand Down Expand Up @@ -2267,13 +2253,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<ISftpFile> InternalListDirectory(string path, Action<int> listCallback)
private IEnumerable<ISftpFile> InternalListDirectory(string path, SftpListDirectoryAsyncResult asyncResult, Action<int> listCallback)
{
if (path is null)
{
Expand Down Expand Up @@ -2309,6 +2296,8 @@ private IEnumerable<ISftpFile> InternalListDirectory(string path, Action<int> li
f.Value));
}

asyncResult?.Update(result.Count);

// Call callback to report number of files read
if (listCallback is not null)
{
Expand Down Expand Up @@ -2375,6 +2364,8 @@ private void InternalDownloadFile(string path, Stream output, SftpDownloadAsyncR

totalBytesRead += (ulong) data.Length;

asyncResult?.Update(totalBytesRead);

if (downloadCallback is not null)
{
// copy offset to ensure it's not modified between now and execution of callback
Expand Down Expand Up @@ -2447,6 +2438,8 @@ private void InternalUploadFile(Stream input, string path, Flags flags, SftpUplo
_ = Interlocked.Decrement(ref expectedResponses);
_ = responseReceivedWaitHandle.Set();
asyncResult?.Update(writtenBytes);
// Call callback to report number of bytes written
if (uploadCallback is not null)
{
Expand Down

0 comments on commit d7688de

Please sign in to comment.