Race Condition with WriteAsync

Question

In BlazorServer app, while uploading a bunch of small files all at once (100 files; 30 - 80k each), I encounter a race condition with the following code:

public async Task UploadAsync() {
    this._fileContent = new MemoryStream();
    using var readStream = OpenReadStream(AppConfiguration.MaxFileSize, cancellationToken);
    byte[] buffer = new byte[30 * 1024];        // Buffer 30k, let's call it "30kb-chunk"
    // ...
    while ((bytesRead = await readStream.ReadAsync(buffer, cancellationToken).ConfigureAwait(false)) != 0) {
        await _fileContent.WriteAsync(buffer, 0, bytesRead).ConfigureAwait(false);
        // ...
    }
    // ...
    // this._fileContent {MemoryStream} gets saved to .zip (using System.IO.Compression.ZipArchive) along with some other stuff; Not relevant here
}

In one example, where files were uploaded "locally" (logged-in to a server & drag&drop locally; upload was super-fast), about 4% of the files were erroneously saved as:

[2nd 30kb-chunk]
[1st 30kb-chunk]
[2nd 30kb-chunk]
... (I guess mix-up could happen with other chunks as well)

Do you create a new class containing the method `UploadAsync()` each time you perform an upload ? — Mad hatter, Aug 31 '23 at 14:42
Yes - the class is a representation of the uploaded file (together with some post-processing), so in example above, 100 instances were created all at once. UploadAsync() was called in each one of them, but there is a limit of max 3 concurrent uploads - next upload starts only when previous finishes with: `Task.Run(async () => { await selectedFiles[index].UploadAsync().ConfigureAwait(false); });` — PetP, Aug 31 '23 at 15:00
I see you're using `index`. Would this variable be used in a `for` loop ? — Mad hatter, Aug 31 '23 at 15:16
Yes, but it's local var introduced inside a for-loop; I'm aware about using direct for-loop-variables in Blazor. Just to make sure that you understand my question, chunks being mixed-up are all from the correct file (not from different files). It's just the order they are written-in that is wrong. It looks almost like "await doesn't have any effect" (if both awaits were to be missing, the race condition would be expected). — PetP, Aug 31 '23 at 15:49
Besides, I don't understand why you are using `ConfigureAwait(false)` everywhere. You can await a `Task` directly. — Mad hatter, Aug 31 '23 at 15:54
For better understanding, I think you should post your code calling `UploadAsync()`, `for` loop included. — Mad hatter, Aug 31 '23 at 15:57
I don't see how code show in the post can have problems you describe because after opening a stream for reading there is no non-local variables that can somehow be impacted by any parallel execution. Please make sure code you've posted is an [mre]. — Alexei Levenkov, Aug 31 '23 at 16:34
I believe this is more specifically an async/await misunderstanding than a race condition. Please read: https://github.com/davidfowl/AspNetCoreDiagnosticScenarios/blob/master/AsyncGuidance.md You specify Blazor, which is ASP.NET Core and does not have a synchronization context, yet you have ConfigureAwait(false). One of your comments shows attempting to consume this function from a Task.Run() which should be avoided. — Adam Vincent, Sep 01 '23 at 03:37
@AdamVincent - FYI - Blazor does have a Synchronisation Context - https://learn.microsoft.com/en-us/aspnet/core/blazor/components/synchronization-context?view=aspnetcore-7.0. The Renderer, which is effectively the "UI", runs in this context — MrC aka Shaun Curtis, Sep 02 '23 at 07:53
Ugh, Thanks for the correction on the SyncronizationContext @MrCakaShaunCurtis — Adam Vincent, Sep 02 '23 at 14:24

MrC aka Shaun Curtis · Answer 1 · 2023-09-02T22:09:02.910

My first step would be to remove all the ConfigureAwait(false).

ConfigureAwait(false) says run the continuation on any available thread. Remove it and the default [true] applies: run the continuation on the same thread.

While ConfigureAwait(false) can give you some performance benefits, it comes with pitfalls if you aren't an async expert. One is introducing parallelism, and consequently behaviour you may not expect. This smells like such behaviour.

Without seeing a lot more of the code, I'm making some educated guesses about the rest of your code. You do talk about using Task.Run, so you are switching Tasks away from the Synchonisation Context and onto the thread pool.

In your code:

bytesRead = await readStream.ReadAsync(buffer, cancellationToken).ConfigureAwait(false)

Says await the ReadAsync and then run the continuation [the write] on any thread. There's no guarantee that write will happen before the next loop read. If you're kicking off a lot file activity, there's no guarantee of the order of the writes: the behaviour you're seeing.

I believe that by removing ConfigureAwait(false), the while loop and all activity will run on the same thread and in the sequence you expect. No guarantees though based on the 6 lines of code in your question.

There's a relevant SO question here:

Usage of ConfigureAwait in .NET

Race Condition with WriteAsync

1 Answers1