Skip to content

Conversation

@Clcanny
Copy link

@Clcanny Clcanny commented Feb 7, 2025

This pull request introduces a minimal yet functional implementation of async_shared_mutex.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 7, 2025
@MaxMotovilov
Copy link
Contributor

MaxMotovilov commented Aug 22, 2025

Sorry for a long delay in review - I have just assumed the co-maintainer's duties and will try to look through PR backlog, among other things.

Questions regarding this PR:

  • IIUC, the goal is to introduce a new functional component similar to a reader/writer lock (can either take out multiple shared locks or one non-shared lock)? It would be nice to include a concise description of the new API somewhere in the PR - ideally in the docs (doc/api_reference.md) :-)
  • I am presently working on merging a PR that reimplements async_mutex in order to make it cancellable. At first glance, your implementation relies on async_mutex directly and should not prevent this desirable trait from propagating to your senders. However
    • You'll need to set the sends_done flag to true (or to the value from async_mutex sender, to be pedantic), and
    • It would be advisable to include unit tests verifying the cancellability of the resulting senders
  • Finally and most importantly - your implementation of async_shared_mutex::try_enqueue() relies on sync_wait() on the embedded async_mutex instance. This looks like a showstopper as it would potentially block the scheduler's thread for an indefinitely long time. Is there an alternative strategy?

@MaxMotovilov MaxMotovilov self-assigned this Aug 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants