3

In a blog post, Microsoft's Sergey Tepliakov says:

you should always provide TaskCreationOptions.RunContinuationsAsynchronously when creating TaskCompletionSource instances.

But in that article, unless I misunderstood it, he also says that all await continuations basically act the same way as a TaskCompletionSource without TaskCreationOptions.RunContinuationsAsynchronously. So this would mean that async and await are also inherently dangerous and shouldn't be used, unless await Task.Yield() is used as well.

(edit: I did misunderstand it. He's saying that await continuing synchronously is the cause of the problematic SetResult behaviour. await Task.Yield() is recommended as a client-side workaround if the task creation can't be controlled at the source. Unless I misunderstood something again.)

I conclude that there must be specific circumstances that make synchronous continuations dangerous, and apparently that those circumstances are common for TaskCompletionSource use cases. What are those dangerous circumstances?

relatively_random
  • 4,505
  • 1
  • 26
  • 48

1 Answers1

6

Thread theft, basically.

A good example here would be a client library that talks over the network to some server that is implemented as a sequence of frames on the same connection (http/2, for example, or RESP). For whatever reason, imagine that each call to the library returns a Task<T> (for some <T>) that has been provided by a TaskCompletionSource<T> that is awaiting results from the network server.

Now you want to write the read-loop that is dequeing responses from the server (presumably from a socket, stream, or pipeline API), resolve the corresponding TaskCompletionSource<T> (which could mean "the next in the queue", or could mean "use a unique correlation key/token that is present in the response").

So you effectively have:

while (communicationIsAlive)
{
    var result = await ParseNextResultAsync(); // next message from the server
    TaskCompletionSource<Foo> tcs = TryResolveCorrespondingPendingRequest(result);
    tcs?.TrySetResult(result); // or possibly TrySetException, etc
}

Now; imagine that you didn't use RunContinuationsAsynchronously when you created the TaskCompletionSource<T>. The moment you do TrySetResult, the continuation executes on the current thread. This means that the await in some arbitrary client code (which can do anything) has now interrupted your IO read loop, and no other results will be processed until that thread relinquishes the thread.

Using RunContinuationsAsynchronously allows you to use TrySetResult (et al) without worrying about your thread being stolen.

(even worse: if you combine this with a subsequent sync-over-async call to the same resource, you can cause a hard deadlock)

Related question from before this flag existed: How can I prevent synchronous continuations on a Task?.

Marc Gravell
  • 1,026,079
  • 266
  • 2,566
  • 2,900
  • So in conclusion: don't complete tasks without RunContinuationsAsynchronously if your code shouldn't be interrupted (SetResult side of the equation), and don't block in task continuations (await side of the equation)? – relatively_random Oct 07 '20 at 10:21
  • 1
    Assuming that you don't mind if your loop gets interrupted, sounds like the only reason to set RunContinuationsAsynchronously on a TaskCompletionSource is to guard against someone continuing your task and then blocking indefinitely. But this is a dangerous practice in asynchronous programming and should never be done anyway. So isn't the advice to "always provide TaskCreationOptions.RunContinuationsAsynchronously" a bit too aggressive? – relatively_random Oct 07 '20 at 10:26
  • 2
    @relatively_random "in conclusion" - effectively, yes; but: it isn't necessarily "indefinitely"; even "for a bit" could be hugely harmful to performance if the read loop is the limiting factor. Re "too aggressive" - well, everything is subjective, but how about we soften it to "use `TaskCreationOptions.RunContinuationsAsynchronously` unless you can explain exactly what it does and why exactly you actively don't want it in your case" :) suggesting it as the default is the safer option, and will almost always lead to the success scenario – Marc Gravell Oct 07 '20 at 10:30