Stream.CopyTo Hangs when used with Librsync.PatchStream

Question

I'm using Librsync in a project to calculate the differences between two versions of a file and apply the changes to the old file.

Outside of my project I got it working in a simple console app that reads the files from 2 different directories, "Patches" them and writes it out to a patched directory.

Code sample -

using (var deltaFile = new FileStream(tmpDeltaFilePath, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None))
{
    //myClient is the client of a WCF service I created
    myClient.ComputeDelta(file.Id, signatureStream).CopyTo(deltaFile);

    originalFile.Seek(0, SeekOrigin.Begin);
    deltaFile.Seek(0, SeekOrigin.Begin);
    var patchedStream = Librsync.ApplyDelta(originalFile, deltaFile);

    using (var patchedFileStream = new FileStream(patchedFilePath, FileMode.OpenOrCreate, FileAccess.ReadWrite, FileShare.None))
    {
        //Code below just hangs. patchedStream pos = 0 and the length is the same as that of the new file.
        patchedStream.CopyTo(patchedFileStream);
    }
}

Pause the debugger and post the call stack of the hand with external code. — usr, Sep 21 '16 at 11:10
Hangs on - `public override int Read(byte[] buffer, int offset, int count) { return ReadAsync(buffer, offset, count).Result; }` in PatchedStream.cs https://github.com/braddodson/librsync.net/blob/master/librsync.net/PatchedStream.cs — chickenbeef, Sep 23 '16 at 13:49
The `.Result` thing looks like a classic ASP.NET deadlock. It's a bug in unknown code. Wrap the entire thing in `Task.Run(() => ...).Wait();` to test this. — usr, Sep 23 '16 at 16:50
I did as you suggested and the file was written to successfully. Is it acceptable/good practice if I leave `Task.Run(() => ...).Wait();` in the code? — chickenbeef, Sep 26 '16 at 10:21

score 2 · Accepted Answer · answered Sep 27 '16 at 09:06

In the comments we found out that this is an instance of the "classic ASP.NET deadlock".

Ideally, you'd find the place where you are using await and are missing ConfigureAwait(false).

I do not believe this to be a bug in the Rsync library. Calling Result on a task is not forbidden if circumstances mandate it. The library is allowed to assume that this will not deadlock. It has no way to check whether this is a safe operation so it must just do it and rely on callers to provide correct objects.

A quick fix is to wrap the Rsync code in Task.Run(() => ...).Wait(); which effectively clears the synchronization context for the duration of this code. Because of that the missing ConfigureAwait(false) has no impact anymore.

This fix usually is acceptable. It has a performance cost because it consumes one more thread and has some synchronization cost. Normally, this is inconsequential. If you take a random ASP.NET app and double the number of threads the chance is high that this has near zero impact. On the upside the fix is clearly correct and easy to maintain.

Stream.CopyTo Hangs when used with Librsync.PatchStream

1 Answers1