Missing ConfigureAwait(false) for ToListAsync and related async terminating operators

[roji]

Our ToListAsync() implementation doesn't do ConfigureAwait(false) like the rest of the EF code. This wasn't flagged by the analyzer, since await foreach specifically currently isn't covered by CA2007 - that's already been fixed (dotnet/roslyn-analyzers#6652) but not yet released.

Flagged by @lkomanetz in #28159 (comment).

[lkomanetz]

@roji
In issue #28159 they talk about ToListAsync deadlocking on a task that is using a blocking .GetAwaiter().GetResult() call. My understanding from that issue was that it was intentional to leave ConfigureAwait(false) off of it because ToListAsync could call into user code and you didn't want to throw the captured context away in case they were doing something with UI elements (or something that requires the captured context).

Did I misunderstand the conversation in that issue? Also I appreciate you getting back to me on it!

[roji]

@lkomanetz it's true that in some very specific (and restricted) flows, we made the decision to preserve the user's synchronization context; but ToListAsync() isn't one of them - ConfigureAwait(false) was just unintentionally left out (e.g. there's ConfigureAwait(false) on ToArrayAsync(), but not on ToListAsync()). In any case, the logic called by ToListAsync() internally does already use ConfigureAwait(false), meaning that the synchronization context is lost anyway.

[lkomanetz]

Thank you for the clarification and for making that change so quickly. I was going to submit a PR through the guidelines but I struggled to think of a way to expose the bug through a unit test, didn't know if it was needed, and felt it might be better to let those who have more intimate knowledge of the codebase implement it.