dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.55k stars 4.54k forks source link

[API Proposal]: AsyncReaderWriterLock #84547

Open tkreindler opened 1 year ago

tkreindler commented 1 year ago

Background and motivation

As we update our applications to asynchronous .Net 6+ based applications one of the most common duplicate patterns I see is that of an async ReaderWriterLock. We have multiple different in-house versions in Substrate alone. The closest thing to an official implementation that I can find is from the Microsoft.VisualStudio.Threading NuGet package but that implementation includes a lot of logic from the rest of that NuGet package. In addition, a dependency on that is yet again another dependency on a NuGet with an irregular release schedule.

I propose a new type AsyncReaderWriterLock (or AsyncReaderWriterLockSlim) which is based on SemaphoreSlim, allowing async reader-writer functionality. I understand the threshold for new types in the runtime is very high but I believe this type would be a great addition to standardizing common patterns.

API Proposal

namespace System.Threading;

// API designed loosely off of ReaderWriterLockSlim and SemaphoreSlim. Additional APIs could be filled in.
public class AsyncReaderWriterLock : IDisposable
{
    public void EnterWriteLock(TimeSpan timeout = default, CancellationToken cancellationToken = default);

    public ValueTask EnterWriteLockAsync(TimeSpan timeout = default, CancellationToken cancellationToken = default);

    public void EnterReadLock(TimeSpan timeout = default, CancellationToken cancellationToken = default);

    public ValueTask EnterReadLockAsync(TimeSpan timeout = default, CancellationToken cancellationToken = default);

    public void ExitReadLock();

    public void ExitWriteLock();
}

API Usage

// global variables
Dictionary<string, string> dictionary = new ();
using AsyncReaderWriterLockSlim lock = new ();

// writer thread/async task
await lock.EnterWriteLockAsync();
try
{
    // do writes under lock
    dictionary["key"] = "value";
}
finally
{
    lock.ExitWriteLock();
}

// reader thread/async task
await lock.EnterReadLockAsync();
try
{
    // do reads under lock
    string value = dictionary["key"];
}
finally
{
    lock.ExitReadLock();
}

Alternative Designs

Alternatively async methods could be added to the existing ReaderWriterLockSlim:

namespace System.Threading;

// partial class adding new methods to the existing class
public partial class ReaderWriterLockSlim : IDisposable
{
    public ValueTask EnterWriteLockAsync(TimeSpan timeout = default, CancellationToken cancellationToken = default);

    public ValueTask EnterReadLockAsync(TimeSpan timeout = default, CancellationToken cancellationToken = default);
}

This would reduce runtime bloat by not adding a new type but I'm not sure the implications it could have on backwards compatibility and performance to the existing type. Most implementations of AsyncReaderWriterLock I've seen have been based on SemaphoreSlim and I'm not sure if jamming an async implementation into the existing class is even feasible.

Risks

The only risk I can think of is the addition of a new type to the runtime causing bloat, but I think the benefits far outweigh this.

ghost commented 1 year ago

Tagging subscribers to this area: @mangod9 See info in area-owners.md if you want to be subscribed.

Issue Details
### Background and motivation As we update our applications to asynchronous .Net 6+ based applications one of the most common duplicate patterns I see is that of an async ReaderWriterLock. We have multiple different in-house versions in Substrate alone. The closest thing to an official implementation that I can find is from the [Microsoft.VisualStudio.Threading](https://learn.microsoft.com/en-us/dotnet/api/microsoft.visualstudio.threading.asyncreaderwriterlock?view=visualstudiosdk-2022) NuGet package but that implementation includes a lot of logic from the rest of that NuGet package. In addition, a dependency on that is yet again another dependency on a NuGet with an irregular release schedule. I propose a new type AsyncReaderWriterLock (or AsyncReaderWriterLockSlim) which is based on SemaphoreSlim, allowing async reader-writer functionality. I understand the threshold for new types in the runtime is very high but I believe this type would be a great addition to standardizing common patterns. ### API Proposal ```csharp namespace System.Threading; // API designed loosely off of ReaderWriterLockSlim and SemaphoreSlim. Additional APIs could be filled in. public class AsyncReaderWriterLock : IDisposable { public void EnterWriteLock(TimeSpan timeout = default, CancellationToken cancellationToken = default); public ValueTask EnterWriteLockAsync(TimeSpan timeout = default, CancellationToken cancellationToken = default); public void EnterReadLock(TimeSpan timeout = default, CancellationToken cancellationToken = default); public ValueTask EnterReadLockAsync(TimeSpan timeout = default, CancellationToken cancellationToken = default); public void ExitReadLock(); public void ExitWriteLock(); } ``` ### API Usage ```csharp // global variables Dictionary dictionary = new (); using AsyncReaderWriterLockSlim lock = new (); // writer thread/async task await lock.EnterWriteLockAsync(); try { // do writes under lock dictionary["key"] = "value"; } finally { lock.ExitWriteLock(); } // reader thread/async task await lock.EnterReadLockAsync(); try { // do reads under lock string value = dictionary["key"]; } finally { lock.ExitReadLock(); } ``` ### Alternative Designs Alternatively async methods could be added to the existing ReaderWriterLockSlim: ```csharp namespace System.Threading; // partial class adding new methods to the existing class public partial class ReaderWriterLockSlim : IDisposable { public ValueTask EnterWriteLockAsync(TimeSpan timeout = default, CancellationToken cancellationToken = default); public ValueTask EnterReadLockAsync(TimeSpan timeout = default, CancellationToken cancellationToken = default); } ``` This would reduce runtime bloat by not adding a new type but I'm not sure the implications it could have on backwards compatibility and performance to the existing type. Most implementations of AsyncReaderWriterLock I've seen have been based on SemaphoreSlim and I'm not sure if jamming an async implementation into the existing class is even feasible. ### Risks The only risk I can think of is the addition of a new type to the runtime causing bloat, but I think the benefits far outweigh this.
Author: tkreindler
Assignees: -
Labels: `api-suggestion`, `area-System.Threading`, `untriaged`
Milestone: -
alexrp commented 1 year ago

Alternatively async methods could be added to the existing ReaderWriterLockSlim:

Note that ReaderWriterLockSlim is thread-affine, so that cannot be done. Adding such methods works for SemaphoreSlim because it doesn't have thread affinity.

teo-tsirpanis commented 1 year ago

that implementation includes a lot of logic from the rest of that NuGet package

Trimming can remove the unrelated logic from the package.

a dependency on that is yet again another dependency on a NuGet

Why is this a bad thing? And moreover it's a Micrsoft package.

with an irregular release schedule

This does not mean it's unsupported.

sakno commented 1 year ago

AsyncReaderWriterLock from .NEXT library which is a member of .NET Foundation. It is mature, optimized (amortized zero alloc of tasks), supports optimistic lock and many other features.