async scope bug

[ccotter]

I know @ispeters is working on #1713, but while I was updating Relacy to support newer versions of GCC (Relacy works by providing its own implementations of things like std::mutex, so Relacy sometimes needs to update its interposing headers with new GCCs), I noticed a bug in async_scope.

If I change the spawn_future test to create the thread before the scope, then we find that the scope can be destructed while the background thread is finishing up handling __complete. In https://github.com/NVIDIA/stdexec/blob/main/include/exec/async_scope.hpp#L162-L163, after the background thread decrements __active_ but before it locks the mutex, the main thread and finish sync waiting on scope.on_empty() then start destructing the scope. Later, the background thread tries to access the now destructed mutex.

I added the sleep to force the bug to manifest, and moved the async_scope to the heap to ensure ASAN notices the use-after-free. The only other change I made was to re-order the static_thread_pool and async_scope instantiations (and mirror their nested destructive order). Without this last change, the bug does not exist since static_thread_pool's destructor joins, ensuring the background thread finishes __complete.

I'm guessing this will be more valuable as a test scenario for @ISpeter's work; not sure if we want to address it in exec::async_scope?

Full ASAN report: https://gist.github.com/ccotter/9fa8761d0d500603aa0e92081386fe9f

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[ericniebler]

after the background thread decrements __active_ but before it locks the mutex

thanks chris! does it fix the issue to lock the mutex first?

[ccotter]

after the background thread decrements __active_ but before it locks the mutex

thanks chris! does it fix the issue to lock the mutex first?

Yes, it does. However, Relacy then gives me yet another bug that I haven't deciphered yet.

[ccotter]

Closing this out with #1734 merged - will look to see if there was another race. I'll also be fixing Relacy to work with newer versions of GCC.

[ccotter]

Yes, it does. However, Relacy then gives me yet another bug that I haven't deciphered yet.

I think the other race Relacy was identifying was the one that #1742 is attempting to fix!