The issue
Currently SemaphoreSlim instances are disposed in a non thread-safe manner. which can lead to major issues. I've encountered the following scenario:
NpgsqlDataSource.Bootstrap() is called, which in turn invokes _setupMappingsSemaphore.WaitAsync()
- At the same time this instance of
NpgsqlDataSource is disposed, which in turn disposes _setupMappingsSemaphore.
- The problem is,
SemaphoreSlim.Dipose() is not thread-safe. In fact, it is entirely possible to dispose a semaphore in such a way that WaitAsync hangs indefinitely. Right here the code awaits a task that could never start if somebody disposes the semaphore in this exact moment.
The unfortunate reality of this scenario is that it's very difficult to determine when an NpgsqlDataSource instance can be safely disposed from a client code due to the fact that some of its methods are internal and invoked indirectly (for example, Bootstrap can be invoked by calling ReloadTypes on an NpgsqlConnection instance).
The proposal
I ask to implement some sort of thread-safe disposal mechanism for SemaphoreSlim instances. I would consider implementing some sort of mechanism that "marks" semaphore instances for disposal and ensures no one can use it while it's being disposed.
The naïve approach would look something like this:
var semaphore = new SemaphoreSlim(1);
if (semaphore.Wait(0))
{
semaphore.Dispose();
}
Unfortunately, this code only mitigates the problem and does not solve it entirely. Somebody can call Release + Wait right after we've blocked the semaphore for disposal.