AsyncLazy can duck tape two callers into the same thread

[lifengl]

When two caller accesses the same AsyncLazy, it can end up with the second caller's continuation code to be executed inside the first caller's stack, which can cause some odd issues (and potentially lead into dead locks.) We found similar issue in CPS's configuredProjectCache, and I think AsyncLazy has a similar issue.

The problem happens, when the AsyncLazy function can finish without yielding the thread. After the first caller creates the value task, it will call resumableAwait.Resume() to execute the task. If the second caller happens to call in during that time, it will chain a task continuation to the value task. When the value task finishes, it will run into the task continuation code of the second task, before returning from the Resume() function. That leads the first task to be blocked by the second task's continuation to finish.

[AArnott]

I see. This doesn't sound like any particular problem with AsyncLazy<T> though, and seems unrelated to the resumableAwait.Resume() technique we're using. It's simply that anyone who shares a Task publicly risks folks attaching synchronous continuations to it, such that when you complete it, those continuations might be inlined.

Maybe there's a race that exists between the first and second caller as to which one adds the first continuation, but either way I think all the continuations will execute inline when the value factory has completed. That causes all continuations to be dependent on the completion of those that came before it (which I'd love to get fixed: https://github.com/dotnet/corefx/issues/34781). Then as you say, if the value factory never yielded, we have one additional dependent added, which is the original callstack that called GetValueAsync.

I suppose we could apply a policy where every continuation is guaranteed to not be inlined on the antecedent. But that would of course come at a perf penalty. We did a lot of work to make sure that in the ideal case of no yielding that we don't add any artificially, so adding artificial yields at this level feels like it would be going backwards.

Is the problem that a continuation then goes and takes a lock, or is it that it blocks on a JTF.Run?

[AArnott]

I think if we get the corefx bug fixed, we'll find some way to fix this one. Either by turning off all inline continuations for this type, or an instance, or perhaps by detecting when a 3rd party gets at the Task before our caller attaches the first one and returning a non-inlining one to that 3rd party.

[lifengl]

If all inline continuations are turned off for this type, we won't have this problem. And I agree the problem won't go away completely (for JTF.Run problem) until the corefx bug is fixed, so this issue is tightly related to the corefx problem. It is, however, slightly different here, because only the second task chained the task continuation, and the first task (which executes the task) hasn't chained the task continuation yet, when the bug happens. Basically, that task is executing the inline resuming code in line 185, and it only await the value task after that on line 193.

[AArnott]

If you're saying that the first caller into GetValueAsync, who executes the value factory, is the second continuation because before appending their continuation, a second thread calls GetValueAsync and gets access to the Task first, yes, I agree a fix in corefx doesn't solve that problem. I can't think of a way to fix that while allowing any continuations to be inlined. And prohibiting inline continuations would require at least two more allocations per AsyncLazy since we'd have to create our own TaskCompletionSource and associated Task to return to our caller with the RunContinuationsAsynchronously flag set. Sounds pretty expensive, but doable. Perhaps we could do so under a flag so we only incur the cost where the instance owner feels it is worth it. And if we apply such a fix, I don't think it would depend on the corefx change since nothing would get inlined.

Do you have places in mind where it would be worth it?