Thread-safe task queue await

Question

I want to do a simple thing (assuming that ContinueWith is thread-safe):

readonly Task _task = Task.CompletedTask;

// thread A: conditionally prolong the task
if(condition)
    _task.ContinueWith(o => ...);

// thread B: await for everything
await _task;

Problem: in above code await _task immediately returns, disregards if there are inner tasks.

Extending requirement that _task can be prolonged by multiple ContinueWith, how would I await for all of them to finish?

---

Of course I can try to do it in old thread-safe way:

Task _task = Task.CompletedTask;
readonly object _lock = new object();

// thread A
if(condition)
    _lock(_lock)
        _task = _task.ContinueWith(o => ...); // storing the latest inner task

// thread B
lock(_lock)
    await _task;

Or by storing tasks in thread-safe collection and using Task.WaitAll.

But I was curious if there is an easy fix to a first snippet?

Answer 1

Your old version is OK, except await while keeping the lock; you should copy that _task to a local variable while holding the lock, release the lock, and only then await

But that ContinueWith workflow is IMO not the best way to implement that logic. ContinueWith was introduced in .NET 4.0, before async-await became part of the language.

In modern C#, I think it’s better to do something like this:

static async Task runTasks()
{
    if( condition1 )
        await handle1();
    if( condition2 )
        await handle2();
}
readonly Task _task = runTasks();

Or if you want them to run in parallel:

IEnumerable<Task> parallelTasks()
{
    if( condition1 )
        yield return handle1();
    if( condition2 )
        yield return handle2();
}
readonly Task _task = Task.WhenAll( parallelTasks() );

P.S. If the conditions are changing dynamically, for the sequential version you should copy their values to local variables, before the first await.

Update: So you are saying your thread A prolongs that task in response to some events which happen dynamically? If yes, your old method is OK, it’s simple and it works, just fix that lock(){ await ... } concurrency bug.

Theoretically, the cleaner approach might be something like a reactor pattern, but it’s way more complicated unfortunately. Here’s my open source example of something slightly similar. You don’t need the concurrency limit semaphore nor the second queue, but you need to expose a property of type Task, initialize it with CompletedTask, replace with a new task created by TaskCompletionSource<bool> from the post method when the first one is posted, and complete that task in runAsyncProcessor method once there’re no more posted tasks.

Answer 2

In your original code await _task; returns immediately because task is completed. You need to do

_task = _task.ContinueWith(...);

You don't have to use locks. You are working in the same thread. In your "locks" version of the code, you do use _task = _task.ContinueWith(...) and that is why it works, not because of the locks.

---

EDIT: Just saw that you want to do _task = _task.ContinueWith(...); in one thread and _task = _task.ContinueWith(...).

This is quite bad as design. You should never combine threads and tasks. You should either go for a task-only solution, or for a thread-only solution.

If you go for a task-only solution (recommended), just create a good sequence of work tasks and then on each task decide how to proceed based on the result of the previous task(s).

If you go for a thread-only solution, you might want to use ManualResetEvent handles to synchronize your tasks.