History log of /haiku/src/system/libroot/os/locks/mutex.cpp
Revision Date Author Comments
# a41f112b 29-Jun-2023 Augustin Cavalier <waddlesplash@gmail.com>

libroot: Downgrade lock assertion into a syslog print.

It seems there has been a bug lurking for years that this exposed.
Rather than inconvenience users further, as it's not especially
hard to trigger, let's down-grade it into a message until it can
be properly investigated and fixed.

Related to #18451.


# 6f3f29c7 06-Jun-2023 Augustin Cavalier <waddlesplash@gmail.com>

user_mutex: Refactor locking and unblocking mechanism.

Suppose the following scenario:

1. Thread A holds a mutex.

2. Thread B goes to acquire the mutex, winds up in kernel waiting.

3. Thread A unlocks; first unsets the LOCKED flag.
As WAITING is set, it calls the kernel; but instead of processing
this immediately, the thread is suspended for any reason (locks,
reschedule, etc.)

4. Thread B hits a timeout, or a signal. It then unblocks in the kernel,
which causes the WAITING flag to be unset.

5. Thread C goes to acquire the lock. It sets the LOCKED flag.
It sees the WAITING flag is not set, so it returns at once,
having successfully acquired the lock.

6. Thread A, suspended back in step 3, resumes.

Now we encounter the problem. Under the previous code, the following
would occur.

7. Thread A sees that no threads are waiting. It thus unsets the LOCKED
flag, and returns from the kernel. Now we have a mutex theoretically
held by thread C but which (illegally) has no LOCKED flag set!

8. Some other thread tries to acquire the lock, and succeeds, for LOCKED
is not set. We now have one lock owned by two separate threads.
That's very bad!

The solution, in this commit, is to (1) switch from using "atomic_or"
to lock mutexes, to using "atomic_test_and_set", and (2) mandate that
_kern_unblock_mutex must be invoked with the mutex already unlocked.

Trying to solve the problem with (2) but without (1) produces other
complications and would overall be more complicated. For instance,
all existing userland code expected that it would set LOCKED, but then
check LOCKED|WAITING. If _kern_mutex_unlock does not unset LOCKED,
then whichever thread sets LOCKED when it was previously unset is
now the mutex's undisputed owner, and if it fails to notice this,
would deadlock.

That could have been solved with extra checks at all lock points, but
then that would mean locks would not be acquired "fairly": it would
be possible for any thread to race with an unlocking thread, and
acquire the lock before the kernel had a chance to wake anyone up.

Given how fast atomics can be, and how slow invoking the kernel is
comparatively, that would probably make our mutexes extremely "unfair."
This would not violate the POSIX specification, but it does seem like
a dangerous choice to make in implementing these APIs.

Linux's "futex" API, which our API bears some similarities to, requires
at least one atomic test-and-set for an uncontended acquisition,
and multiple atomics more for even the simplest case of contended
acquisition. If it works for them, it should work for us, too.

Fixes #18436.

Change-Id: Ib8c28acf04ce03234fe738e41aa0969ca1917540
Reviewed-on: https://review.haiku-os.org/c/haiku/+/6537
Tested-by: Commit checker robot <no-reply+buildbot@haiku-os.org>
Reviewed-by: Adrien Destugues <pulkomandy@pulkomandy.tk>
Reviewed-by: waddlesplash <waddlesplash@gmail.com>


# 65a76a0f 06-Jun-2023 Augustin Cavalier <waddlesplash@gmail.com>

pthread & os/locks: Add some more assertions and error checks.

The first of these assertions in the pthread code is actually possible
to trigger under some specific circumstances, which is ticket #18436.
This makes that problem more obvious when it does happen.

Change-Id: I026ea6e4c569a7c20d82b70722f752d87e57c5a1
Reviewed-on: https://review.haiku-os.org/c/haiku/+/6536
Reviewed-by: waddlesplash <waddlesplash@gmail.com>
Tested-by: Commit checker robot <no-reply+buildbot@haiku-os.org>
Reviewed-by: Adrien Destugues <pulkomandy@pulkomandy.tk>


# b916156a 13-Jul-2014 Julian Harnath <julian.harnath@rwth-aachen.de>

Move libroot synchronization functions to private namespace

* Prefix lock functions with __ to mark them as private. Add
forwarding macros to keep existing code working.

* Avoids symbol name clashes with kernel lock APIs, occuring when
using kernellandemu-lib in userlandfs. Thanks to Ingo for the
suggestion.


# 82727571 11-Dec-2013 Pawel Dziepak <pdziepak@quarnos.org>

libroot: Fix uninitialized counter in mutex_lock()


# 7e1c4534 07-Nov-2013 Pawel Dziepak <pdziepak@quarnos.org>

libroot: Add adaptive mutex implementation


# f7127458 15-Apr-2010 Ingo Weinhold <ingo_weinhold@gmx.de>

* Private libroot locking primitives:
- Reimplemented mutex to use the _kern_mutex*() syscalls.
- The initializer functions cannot fail anymore -- changed their return type
to void.
- Changed the initializer function semantics to not copy the name by default
anymore (as in the kernel). Also added *_etc() versions of them that take an
additional flags.
- Added static initializer macros.
- Made the mutex (and thus recursive_lock) lock functions non-interruptable.
- Got rid of the "lazy" version. They are no longer needed, since the
initialization of the standard types can be done statically and cannot fail.
* Adjusted libroot, runtime loader, and other code using the private libroot
locking primitives to the new semantics.
* pthreads mutexes and condition variables:
- Reimplemented using the _kern_mutex*() syscalls.
- Consistently use POSIX error codes.
- Fixed some not quite POSIX compliant behavior.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@36323 a95241bf-73f2-0310-859d-f6bbb57e9c96


# 258b34c5 30-Nov-2009 Ingo Weinhold <ingo_weinhold@gmx.de>

* Fixed wrong parameter of lazy_mutex_destroy().
* Split locks.cpp into mutex.cpp, recursive_lock.cpp, and rw_lock.cpp (new
subdirectory locks/).
* runtime_loader no longer includes the rw_lock, allowing removal of the TLS
dependency again.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@34364 a95241bf-73f2-0310-859d-f6bbb57e9c96


# b916156a835cd4f1c16990c33d69190014df77b7 13-Jul-2014 Julian Harnath <julian.harnath@rwth-aachen.de>

Move libroot synchronization functions to private namespace

* Prefix lock functions with __ to mark them as private. Add
forwarding macros to keep existing code working.

* Avoids symbol name clashes with kernel lock APIs, occuring when
using kernellandemu-lib in userlandfs. Thanks to Ingo for the
suggestion.


# 82727571c323b109214bb78496740b9b4b861a12 11-Dec-2013 Pawel Dziepak <pdziepak@quarnos.org>

libroot: Fix uninitialized counter in mutex_lock()


# 7e1c4534df199347d63bf7235ced8cc60170bfa7 07-Nov-2013 Pawel Dziepak <pdziepak@quarnos.org>

libroot: Add adaptive mutex implementation


# f71274580bc2625bf438140839fe38193ece28e6 15-Apr-2010 Ingo Weinhold <ingo_weinhold@gmx.de>

* Private libroot locking primitives:
- Reimplemented mutex to use the _kern_mutex*() syscalls.
- The initializer functions cannot fail anymore -- changed their return type
to void.
- Changed the initializer function semantics to not copy the name by default
anymore (as in the kernel). Also added *_etc() versions of them that take an
additional flags.
- Added static initializer macros.
- Made the mutex (and thus recursive_lock) lock functions non-interruptable.
- Got rid of the "lazy" version. They are no longer needed, since the
initialization of the standard types can be done statically and cannot fail.
* Adjusted libroot, runtime loader, and other code using the private libroot
locking primitives to the new semantics.
* pthreads mutexes and condition variables:
- Reimplemented using the _kern_mutex*() syscalls.
- Consistently use POSIX error codes.
- Fixed some not quite POSIX compliant behavior.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@36323 a95241bf-73f2-0310-859d-f6bbb57e9c96


# 258b34c594867f4447c4e3b5ce9240075f48b7f4 30-Nov-2009 Ingo Weinhold <ingo_weinhold@gmx.de>

* Fixed wrong parameter of lazy_mutex_destroy().
* Split locks.cpp into mutex.cpp, recursive_lock.cpp, and rw_lock.cpp (new
subdirectory locks/).
* runtime_loader no longer includes the rw_lock, allowing removal of the TLS
dependency again.


git-svn-id: file:///srv/svn/repos/haiku/haiku/trunk@34364 a95241bf-73f2-0310-859d-f6bbb57e9c96