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

AsyncMonitor.WaitAsync is not picking up the pulse. #33

Closed
niemyjski opened this issue Sep 15, 2015 · 4 comments
Closed

AsyncMonitor.WaitAsync is not picking up the pulse. #33

niemyjski opened this issue Sep 15, 2015 · 4 comments
Assignees

Comments

@niemyjski
Copy link

I created a unit test to try and understand how it works.. I came across a scenario that should work (reread your docs and others for a good 15 minutes).

        [Fact]
        public async Task WillWaitForMonitor() {
            var monitor = new AsyncMonitor();

            // 1. Fall through with the delay.
            var sw = Stopwatch.StartNew();
            using(await monitor.EnterAsync())
                await Task.WhenAny(Task.Delay(100), monitor.WaitAsync()).AnyContext();
            sw.Stop();
            Assert.InRange(sw.ElapsedMilliseconds, 100, 125);
            // 2. Wait for the pulse to be set.
            sw.Restart();
            using (await monitor.EnterAsync()) {
                Task.Run(async () => {
                    await Task.Delay(50).AnyContext();
                    Logger.Trace().Message("Pulse").Write();
                    monitor.Pulse();
                    Logger.Trace().Message("Pulsed").Write();
                });

                Logger.Trace().Message("Waiting").Write();
                await monitor.WaitAsync().AnyContext();
            }

            sw.Stop();
            Assert.InRange(sw.ElapsedMilliseconds, 50, 100);
        }

It outputs

[15:51:59.602 T InMemoryLockTests] Waiting
[15:51:59.654 T InMemoryLockTests] Pulse
[15:51:59.657 T InMemoryLockTests] Pulsed

But the monitor.WaitAsync() never completes and waits for ever. Is there a reason for this?

@StephenCleary
Copy link
Owner

Monitor.WaitAsync is a method that changes the underlying state of the monitor. You especially need to be careful when using Task.WhenAny with synchronization primitives (there is almost never a good reason to do this).

In this case, as far as the monitor knows, it has two waiters: the first WaitAsync and the second WaitAsync. So when the Pulse comes through, it satisfies the first task, as it should.

FYI, the unit test for "pulsing a monitor releases a waiter" is here.

On a side note, unit tests that depend on timings are inherently unstable.

@niemyjski
Copy link
Author

Thanks for the heads up. I found that I needed to pass a cancellation token to the waiter and let it blow up (catch it)

@niemyjski
Copy link
Author

I see in your unit tests that you are entering the monitor to pulse. In your docs and my working unit test it doesn't mention that you have to do this. Is this required or just a best practice?

@StephenCleary
Copy link
Owner

Yes, it is required. I'll update the docs.

@StephenCleary StephenCleary self-assigned this Jun 30, 2016
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