libc: implement pthread_mutex_timedlock and pthread_mutex_clocklock - #1426
libc: implement pthread_mutex_timedlock and pthread_mutex_clocklock#1426gburd wants to merge 1 commit into
Conversation
Both were stubs returning EINVAL, so code that uses a bounded mutex acquire (common in databases and glibc-based libraries) could not run. OSv's lockfree mutex has no native timed acquire, so implement them by polling try_lock() with a short (50 us) sleep between attempts until the absolute deadline, returning 0 on success or ETIMEDOUT when the deadline passes. The uncontended case takes the try_lock() fast path with no sleeping. pthread_mutex_clocklock supports CLOCK_REALTIME and CLOCK_MONOTONIC (converting the deadline against the right clock) and returns EINVAL for any other clock; a malformed timespec also returns EINVAL. This is a poll-and-backoff acquire rather than a native timed wait, which is fine for the rare contended timedlock; the lockfree mutex could grow a real timed wait later if it ever needs to be tight. Add tests/tst-pthread-timedlock.cc: uncontended success, contended timeout (ETIMEDOUT from another thread while the main thread holds the lock, no deadlock), success after unlock, CLOCK_MONOTONIC clocklock, and the EINVAL clock path. Passes on OSv under KVM; tst-pthread continues to pass.
nyh
left a comment
nyh
left a comment
There was a problem hiding this comment.
I think I'll accept it. Obviously, the "proper" fix would be to add a timeout to OSv's native lockfree mutex implementation (lfmutex.cc, see also big comment in mutex.hh). But that would be far from trivial. The main difficulty is after a waited inserted himself in the waiting list, how to "remove" this waiter from this list when it gave up on the wait (because of the timeout).
There may be ways to solve that - perhaps by keeping "canceled" waits alive somewhere (for the unlock to skip), or by replacing the queue_mpsc by some sort of data structure that allows producers to remove items from the queue. Of course the difficult part is how to do that without using a spin-lock (i.e., a "lockfree" algorithm).
We definitely don't need to do this now, and I agree to take your "ponytail" (cute slang ;-)) implementation for now.
I just had with my AI an interesting chat on how to do this. I raised a lot of interesting ideas, I'll open an issue about it for future reference.
| // so we poll try_lock() with a short sleep between attempts. Returns 0 on | ||
| // success or ETIMEDOUT if the deadline passes first. | ||
| // | ||
| // ponytail: poll+backoff, not a native timed acquire. Fine for the rare |
There was a problem hiding this comment.
Ponytail? AI is now inventing slang? :-) :-)
|
By the way, Fixes #834 |
There was a problem hiding this comment.
Pull request overview
This PR implements pthread_mutex_timedlock() and pthread_mutex_clocklock() in OSv’s libc, replacing stubbed EINVAL returns so software relying on bounded mutex acquisition (e.g., databases and glibc-oriented libraries) can run correctly.
Changes:
- Add a poll-and-backoff timed mutex acquire helper used by both
pthread_mutex_timedlockandpthread_mutex_clocklock. - Support
CLOCK_REALTIMEandCLOCK_MONOTONICdeadlines forpthread_mutex_clocklock, rejecting unsupported clocks withEINVAL. - Add a new test (
tst-pthread-timedlock) and wire it into the tests module build.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/tst-pthread-timedlock.cc | Adds coverage for uncontended success, contended timeout, post-unlock success, monotonic clocklock, and unsupported clock behavior. |
| modules/tests/Makefile | Registers the new tst-pthread-timedlock.so test target in the test suite. |
| libc/pthread.cc | Implements timed mutex locking via try_lock() polling with short sleeps, and adds pthread_mutex_clocklock() clock handling. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (from_libc(m)->try_lock()) { | ||
| return 0; // uncontended fast path | ||
| } | ||
| if (!abs_timeout || abs_timeout->tv_nsec < 0 || | ||
| abs_timeout->tv_nsec >= 1000000000) { | ||
| return EINVAL; | ||
| } |
There was a problem hiding this comment.
I think this is minor enough that I'll merge your patch as-is, and let you fix this (and consider if you want to change or keep the cute AI-ish slang "ponytail") in a followup patch.
| // ponytail: poll+backoff, not a native timed acquire. Fine for the rare | ||
| // contended timedlock; if it ever needs to be tight, add a timed wait to the | ||
| // lockfree mutex itself. |
|
Thanks for merging, and good catches. Follow-up on branch
|
What
pthread_mutex_timedlockandpthread_mutex_clocklockwere stubs returningEINVAL, so code that uses a bounded mutex acquire (common in databases andglibc-based libraries) could not run.
How
OSv's lockfree mutex has no native timed acquire, so these poll
try_lock()with a short (50 us) sleep between attempts until the absolute deadline,
returning 0 on success or
ETIMEDOUTwhen the deadline passes. The uncontendedcase takes the
try_lock()fast path with no sleeping.pthread_mutex_clocklocksupportsCLOCK_REALTIMEandCLOCK_MONOTONIC(converting the deadline against the right clock) and returns
EINVALfor anyother clock; a malformed timespec also returns
EINVAL.This is a poll-and-backoff acquire rather than a native timed wait, which is
fine for the rare contended timedlock; the lockfree mutex could grow a real
timed wait later if it ever needs to be tight.
Testing
tests/tst-pthread-timedlock.cc: uncontended success, contended timeout(
ETIMEDOUTfrom another thread while the main thread holds the lock, nodeadlock), success after unlock,
CLOCK_MONOTONICclocklock, and theEINVALclock path. Passes on OSv under KVM;
tst-pthreadcontinues to pass (10/10).