Fix Async thread blocking on SqlConnection open for AAD modes

[cheenamalhotra]

Fixes #1209

Breaking Change [Low impact]:

• The driver now throws SqlException replacing AggregateException for Active Directory Authentication modes.

[Wraith2]

/cc @roji for async expertise

[Wraith2]

Try ConfigureAwait(false) at all awaits to prevent the task scheduler trying make tasks continue on the thread that started them. In this library (Because it's a library) all awaits should be ConfigureAwait(false).

[vonzshik]

Try ConfigureAwait(false) at all awaits to prevent the task scheduler trying make tasks continue on the thread that started them. In this library (Because it's a library) all awaits should be ConfigureAwait(false).

Doesn't really matter in that specific case because Task.Run ensures that the delegate is run on the threadpool, which lacks the synchronization context.

[roji]

Yeah, this seems like the classical block-over-async deadlock (see this article by Stephen Cleary). Changing Task.Run should indeed resolve the Winforms deadlock, since the code runs without the Winforms SynchronizationContext.

However, this is still sync-over-async and therefore strongly discouraged, since it can cause various starvation/pseudo-deadlock effects. For example, if an application starts up and attempts to establish lots of connections simultaneously ("thundering herd"), this code would be executed many times in parallel. Each method would enqueue AcquireTokenAsync to the thread pool, and then block the current thread. If the current thread itself happens to be a TP thread (e.g. ASP.NET), the thread pool could quickly get exhausted, as its threads are all blocking synchronously waiting for AcquireTokenAsync to complete, but there are no available TP threads to execute it. The TP will hill-climb and create new threads - so this will eventually get resolved, and therefore isn't a true deadlock - but it's quite a problematic effect.

If authProvider doesn't provide a sync API for acquiring the token, then it's reasonable to throw NotSupportedException if a user tries to synchronously open a connection that needs this. Since I'm assuming that's not possible, another way around this is to have a private thread pool (e.g. of size 1), and use a SynchronizationContext to make sure continuations are executed on that pool. This would guarantee that AcquireTokenAsync never waits for execution because the regular TP is exhausted. For an example of Npgsql doing this internally, see this.