dotnet / dotNext

Next generation API for .NET
https://dotnet.github.io/dotNext/
MIT License
1.56k stars 119 forks source link

Add `IDisposable`-returning extension method for upgrading read lock to write lock #241

Closed cmeeren closed 1 week ago

cmeeren commented 4 weeks ago

In the docs on Asynchronous locks, section "Built-in Reader/Writer Synchronization", it says that you can use AcquireReadLockAsync and AcquireWriteLockAsync on any reference type.

Could you consider adding a similar IDisposable-returning extension method for upgrading from read to write?

sakno commented 3 weeks ago

It's a bit problematic due to overload resolution. AcquireXXX extension methods do not clash with instance methods in AsyncReaderWriterLock. If I add UpgradeToWriteLockAsync extension method, it will hide instance method with the same name. If you need disposable-like object, use AsyncLock.WriteLock(AsyncReaderWriterLock, bool) static method.

cmeeren commented 3 weeks ago

As I understand it, it's just a naming issue, then? Since all the IDisposable-returning methods are called Acquire..., how about just following that pattern and calling it AcquireUpgradeToWriteLockAsync? It's a bit weird, but not that much, and in the light of other similar method names, I'd say it's pretty clear.

sakno commented 3 weeks ago

AcquireUpgradeToWriteLockAsync contains two verbs in the beginning, which is not good.

cmeeren commented 3 weeks ago

Upgrade is also a noun: "Acquire an upgrade to a write lock".

sakno commented 3 weeks ago

Or I can add the following overload:

static ValueTask<AsyncLock.Holder> AcquireWriteLockAsync<T>(this T obj, bool upgrade, CancellationToken token = default)
cmeeren commented 3 weeks ago

Great, thanks!

By the way, is it possible using this library to either acquire a write lock or to upgrade an existing read lock? I know there is a property that indicates whether or not a read lock is currently held, but that is explicitly documented as being used to monitor state, not for synchronization purposes.

sakno commented 3 weeks ago

is it possible using this library to either acquire a write lock or to upgrade an existing read lock?

No, and it is intended. Upgrading to write lock is a dangerous operation that may lead to dead lock. Check #236 discussion.