Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Discussion] Return ValueTask instead of AwaitableDisposable? #265

Open
BalassaMarton opened this issue Oct 14, 2022 · 5 comments
Open

[Discussion] Return ValueTask instead of AwaitableDisposable? #265

BalassaMarton opened this issue Oct 14, 2022 · 5 comments
Assignees

Comments

@BalassaMarton
Copy link

Methods returning AwaitableDisposable could return ValueTask instead.

Pros

  • Much faster and allocates less on the happy-path (when the lock is not already taken).

Cons

  • Massively breaking change due to the limitations of ValueTask.
  • Not compatible with older frameworks.

I'll submit an experimental PR just for the sake of discussion.

@modmynitro
Copy link

Could the System.Threading.Tasks.Extensions nuget package be used to support older framworks.

@StephenCleary StephenCleary self-assigned this Mar 8, 2023
@NotTsunami
Copy link

Could the System.Threading.Tasks.Extensions nuget package be used to support older framworks.

Yes, it should be possible to use ValueTask from System.Threading.Tasks.Extensions to support older frameworks.

Pretty late to the discussion, but this PR as a whole would be pretty interesting to expand on.

@BalassaMarton
Copy link
Author

Created a new experimental async lock implementation that supports fast, zero-alloc reentrancy: https://github.com/BalassaMarton/AsyncExtensions
I'd be happy to see your feedback, especially from @StephenCleary

@timcassell
Copy link

@BalassaMarton I'm always skeptical of any re-entrant async locks. Does your solution hold up to the problems presented in this article?
I even tried my hand at the problem myself, but abandoned the idea. (But I do have an alloc-free non-re-entrant async lock in my library.)

@BalassaMarton
Copy link
Author

BalassaMarton commented Apr 12, 2024

@timcassell look at this example: https://github.com/BalassaMarton/AsyncExtensions?tab=readme-ov-file#example-recursive-async-locking-using-a-token
The specific problem I was trying to solve: I had a bunch of short, specific methods that needed exclusive access to resources, but it was hard to follow which methods were doing the actual locking vs assuming the lock being already taken. With this approach, the method parameter effectively documents that it will asynchronously lock on the resource if it's not locked already:

async ValueTask OuterMethod(AsyncLockToken token = default)
{
    using (token = await mutex.LockAsync(token))
    {
        await InnerMethod(token);
    }
}

async ValueTask InnerMethod(AsyncLockToken token = default)
{
    using (await mutex.LockAsync(token))
    {
        // ...
    }
}

I even had a variation where you could name your AsyncLock objects and annotate any method that will lock on them like [LocksOn("lockName")] - and LockAsync would validate that methods in the current call stack are correctly annotated. This is of course all experimental, I'm not saying any of this is the way to do async locking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants