timcassell / ProtoPromise

Robust and efficient library for management of asynchronous operations in C#/.Net.
MIT License
173 stars 13 forks source link

`TryWait` with an already canceled token does not release the lock. #226

Closed timcassell closed 1 year ago

timcassell commented 1 year ago

If the token is already canceled, the lock should be released, then re-enter the lock queue, in case another part of code is already waiting to acquire the lock, in which case the wait should receive the signal and return true. The behavior should match Monitor.Wait(obj, 0); This test fails:

[Test]
public void AsyncConditionVariable_TryWaitAsync_AlreadyCanceled_AnotherLockWaiting_ReturnsTrue()
{
    var mutex = new AsyncLock();
    Promise.Run(async () =>
    {
        Promise notifyPromise;
        using (var key = await mutex.LockAsync())
        {
            notifyPromise = Promise.Run(async () =>
            {
                using (var key2 = await mutex.LockAsync())
                {
                    AsyncMonitor.Pulse(key2);
                }
            }, SynchronizationOption.Synchronous);

            var success = await AsyncMonitor.TryWaitAsync(key, CancelationToken.Canceled());
            Assert.True(success);
        }
        return notifyPromise;
    }, SynchronizationOption.Synchronous)
        .WaitWithTimeoutWhileExecutingForegroundContext(TimeSpan.FromSeconds(1));
}
timcassell commented 1 year ago

After testing Monitor.Wait, I found that my assumption was only half-true. Running this test, it randomly returns true or false. After looking into why, it seems that's due to the granularity of the system clock. According to the documentation, it should always return false if timeout is 0, but due to the system clock, it may return true.

[Test]
public void Monitor_WaitPulse()
{
    var mutex = new object();
    var promise = Promise.Run(() =>
    {
        lock (mutex)
        {
            Monitor.Pulse(mutex);
        }
        Thread.Sleep(100);
        lock (mutex)
        {
            Monitor.Pulse(mutex);
        }
    }, SynchronizationOption.Background, forceAsync: true);

    lock (mutex)
    {
        Monitor.Wait(mutex);
    }
    lock (mutex)
    {
        Thread.Sleep(1000);
        Assert.True(Monitor.Wait(mutex, 0));
    }

    promise.WaitWithTimeout(TimeSpan.FromSeconds(1));
}

So, I will close this as working as intended.