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

Enforce that monitor is entered when calling Pulse and PulseAll or remove requirement? #260

Open
WalkerCodeRanger opened this issue Aug 17, 2022 · 1 comment
Assignees

Comments

@WalkerCodeRanger
Copy link

The doc comments for AsyncMonitor.Pulse() and AsyncMonitor.PulseAll() say "The monitor MUST already be entered when calling this method." It doesn't explain what will actually happen if that isn't done.

Testing shows that calling Pulse or PulseAll outside of the monitor does actually release any waiters. I'm not sure what the requirement that it must be entered is about. It seems like changes should be made in line with one of three options:

  1. If there isn't actually a requirement to enter the monitor, then update the docs and doc comments to reflect that.
  2. If there is actually a requirement to enter the monitor
    • check that the monitor has been entered and throw an exception if it hasn't (the System.Threading.Monitor class throws SynchronizationLockException when Pulse or PulseAll is called when the calling thread does not own the lock for the specified object. I understand that doesn't make sense for AsyncMonitor because it doesn't support reentrancy, but there should be some equivalent check.
    • update the doc comments to explain why this restriction is in place
    • if possible enforce that Pulse and PulseAll are only called from the code the entered the monitor. The way to do this would be to move them from AsyncMonitor to disposable object returned by EnterAsync
  3. If there is actually a requirement to enter the monitor, but nothing can be done to enforce this
    • update the doc comments to explain why this restriction is in place and what will go wrong if it isn't followed.
@StephenCleary
Copy link
Owner

It can't be enforced via exceptions; it would be possible but too expensive in the async world. Enforcing by API is not a bad idea, though.

Thanks for writing this up!

@StephenCleary StephenCleary self-assigned this Aug 18, 2022
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

2 participants