roc-streaming / roc-toolkit

Real-time audio streaming over the network.
https://roc-streaming.org
Mozilla Public License 2.0
1.06k stars 213 forks source link

Get rid of unnecessary guards in Mutex, Cond, and Semaphore #361

Open gavv opened 4 years ago

gavv commented 4 years ago

Last revised: Oct 2023

Into

There is known issue in various implementation of POSIX mutexes, condition variables, and semaphores, which is well described here: https://sourceware.org/bugzilla/show_bug.cgi?id=12674

TLDR: when you call semaphore.post(), another thread may wake up before post() returns, and if it tries to destroy the semaphore, you can get a crash or error.

This issue is present on old glibc and musl version (for mutexes, cond vars, and semaphores) and, AFAIK, on all macOS versions (at least for cond vars, and likely for other primitives too). We encountered it on macOS in Roc. I suspect that it may be present on other platforms as well, since it's quite tricky to implement those primitives without this problem.

We have implemented a workaround for this issue in this commit: 608b7333d4c4268391ca576511887c2908d6e9e8

The workaround is enabled for core::Mutex, core::Cond, and POSIX-based implementation of core::Semaphore.

It's not needed for macOS implementation of core::Semaphore, since on macOS we use mach semaphores instead of POSIX semaphores, which are kernel objects and don't have such a problem.

However, on one hand the workaround is not free (it makes post(), unlock(), or signal() operations heavier, as well as destructors), and on the other hand it's not needed on all platforms. At least, it not needed on recent glibc or musl.

Task

This task suggests to disable the workaround linked above (guard_ atomic) in core::Mutex, core::Cond, and core::Semaphore when we're using a libc which is known not to have the described problem.

According to libuv sources, the issue was fixed in glibc 2.21. We should also determine the uclibc version with the similar fix. We could also check if this problem present on bionic (android libc). If you're aware of other platforms that don't have this problems, or have access to them and can test it, you're welcome.

Then we should disable guard when using "good" versions of glibc and uclibc. Ideally this should be a compile-time check, when possible.