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

async_mutex::unlock() behavior should be better documented #204

Open
aschultz opened this issue Jul 2, 2021 · 2 comments
Open

async_mutex::unlock() behavior should be better documented #204

aschultz opened this issue Jul 2, 2021 · 2 comments

Comments

@aschultz
Copy link

aschultz commented Jul 2, 2021

async_mutex::unlock() makes a direct call to handle.resume() on the next awaiter in the queue. This prevents the code releasing the mutex from continuing its execution until the next awaiter -- potentially all the awaiters -- have completed. (Completion here meaning either naturally or by co_awaiting an operation that schedules the remaining work for later execution).

It also means the thread an awaiter resumes on is always the thread of the first lock holder in the chain.

static async_mutex s_mutex{};

IAsyncAction DoWork() {
    {
        auto scopedLock = co_await s_mutex.lock_async();
        // (A) Do work that needs the mutex
    }
    // Lock is destroyed, calling s_mutex.unlock(); 
    // The next queued call to DoWork starts running. We are now blocked here until it completes.

    // (B) This line doesn't run until every queued DoWork is processed.
}

This behavior isn't intuitive. Its unusual for a future operation to have an impact on the current one. I would have expected instead that the code calling unlock() queues the next awaiter through some scheduler (thread pool, UI queue, etc), allowing the current operation to run through (B) immediately.

I realize the current design is more agnostic to the environment and devs can still customize where execution continues. So I'd ask to at least improve the documentation here to make clear 1) the impact of this design and 2) that you may want a call to something like winrt::resume_* after the lock_async() call to avoid blocking previous operations.

@Maximsiv1410
Copy link

What's more - what if resumed coroutine throws an exception?
async_mutex_lock releases async_mutex in destructor via calling mutex->unlock()
So there is potential undefined behavior

@jeanga
Copy link

jeanga commented Apr 11, 2022

Please be aware of #208 .

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

3 participants