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 Nov 1, 2023
1 parent 508fc87 commit 6b1595b
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 @@ -583,7 +583,7 @@ public IEnumerable<ISftpFile> ListDirectory(string path, Action<int> listCallbac
{
CheckDisposed();

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

/// <summary>
Expand Down Expand Up @@ -666,12 +666,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 @@ -898,12 +893,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 @@ -1131,11 +1121,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 List<FileInfo> InternalSynchronizeDirectories(string sourcePath, string

#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 @@ -2268,13 +2254,14 @@ private List<FileInfo> InternalSynchronizeDirectories(string sourcePath, string
/// 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 <see langword="null"/>.</exception>
/// <exception cref="SshConnectionException">Client not connected.</exception>
private List<ISftpFile> InternalListDirectory(string path, Action<int> listCallback)
private List<ISftpFile> InternalListDirectory(string path, SftpListDirectoryAsyncResult asyncResult, Action<int> listCallback)
{
if (path is null)
{
Expand Down Expand Up @@ -2314,6 +2301,8 @@ private List<ISftpFile> InternalListDirectory(string path, Action<int> listCallb
f.Value));
}

asyncResult?.Update(result.Count);

// Call callback to report number of files read
if (listCallback is not null)
{
Expand Down Expand Up @@ -2380,6 +2369,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 @@ -2452,6 +2443,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 6b1595b

Please sign in to comment.