Skip to content

fix(libkernel): treat a null deadline as an untimed lock in the timed pthread locks - #7035

Merged
oneandonlydean merged 2 commits into
boykopovar:mainfrom
milescoviello:fix/timed-locks-null-deadline
Oct 17, 2026
Merged

oneandonlydean merged 2 commits into
boykopovar:mainfrom
milescoviello:fix/timed-locks-null-deadline

Conversation

@milescoviello

@milescoviello milescoviello commented Oct 17, 2026 •

Copy link
Copy Markdown
Contributor

What

pthread_rwlock_timedrdlock, pthread_rwlock_timedwrlock and pthread_mutex_timedlock threw std::runtime_error (null abstime) for a null deadline. In FreeBSD 11's libthr, a null abstime reaches the untimed kernel lock:

The three calls now do what pthread_rwlock_rdlock, pthread_rwlock_wrlock and pthread_mutex_lock do when abstime is null.

I don't know of a title that hits this. I found it while comparing the pthread wrappers with libthr.

Tested

Ubuntu 24.04, GCC 13.3, Release, on main 6a32550.

@screamm rechecked c7849f849 on Windows: guest_pthread_timed_locks passes 20 of 20 direct runs on a loaded host (where it had aborted 15 of 20) and 20 of 20 next to an 8-job build; both tests abort with main's Mutex.cpp and Rwlock.cpp.

Not checked: macOS, a console.

Checklist

  • Based on current main; no other open PR implements the same functions
  • One topic per PR; follow-ups go in a new PR
  • No comments in code except technical debt
  • Unimplemented paths throw (NotImplemented_nid_no_patch); silent stubs are listed under Silent stubs in their library's technical debt file
  • No notes, investigation .md files or images added to the repository (attach them to this PR)
  • New third-party code is a submodule built from source
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes

🤖 Generated with Claude Code

… pthread locks

pthread_rwlock_timedrdlock, pthread_rwlock_timedwrlock and
pthread_mutex_timedlock threw for a null abstime. FreeBSD's libthr
passes a null abstime on to the untimed kernel lock, so these calls
then wait like pthread_rwlock_rdlock, pthread_rwlock_wrlock and
pthread_mutex_lock.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Oct 17, 2026 •

Copy link
Copy Markdown

Other open pull requests touch the same code. Merging one will break the other, or both implement the same thing. If this one builds on them, list them in Depends on.

Pull request Conflicting files (lines on main) Same exports Same new files
PR 6241 core/libs/prx/libkernel/CMakeLists.txt L671

Some comparisons could not be completed because a pull request conflicts with the base branch. Review these shared files manually before concluding that there is no overlap.

Pull request Shared files not compared
PR 4972 core/libs/prx/libkernel/CMakeLists.txt
PR 5021 core/libs/prx/libkernel/CMakeLists.txt
PR 6044 core/libs/prx/libkernel/CMakeLists.txt
core/libs/tests/GuestPthreadRwlock.cpp
PR 6222 core/libs/prx/libkernel/CMakeLists.txt
PR 6287 core/libs/prx/libkernel/CMakeLists.txt
PR 6487 core/libs/prx/libkernel/CMakeLists.txt

@screamm

screamm commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Windows run in #7023 (comment): both tests pass 3/3, and with the source reverted both throw "null abstime". guest_pthread_timed_locks aborted in 15 of 20 runs on a heavily loaded host and passed 20 of 20 on a quieter one. Every abort was the elapsed >= HoldTime / 2 check, after the main thread saw held 40-90 ms late; the lock was always taken after the holder unlocked. A flag the holder sets just before unlocking, checked when acquire() returns, would make the test deterministic.

Written with Claude Code assistance (validation by a subagent, reviewed in this session).

…not elapsed time

The waiter took the lock only after the holder unlocked, but under load it
saw the holder's lock late and measured less than half the hold time.
The holder now sets a flag just before it unlocks and the waiter requires
it once its lock is taken.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@milescoviello

Copy link
Copy Markdown
Contributor Author

Thanks @screamm, that diagnosis was right. The lock was taken only after the holder unlocked, so the half-hold-time check was measuring scheduler delay. c7849f8 does what you suggested: the holder sets releasing just before it unlocks, and the waiter requires that flag once acquire() returns. Here, under 24 CPU burners, the new test passes 20 of 20, and both lock tests pass 3 of 3 with ctest. (The old check also passed 20 of 20 under that load on Linux, so I couldn't reproduce the abort here.) I added your Windows run to the description.

AI-assisted: yes (Claude Code)

@screamm

screamm commented Oct 17, 2026

Copy link
Copy Markdown
Contributor

Rechecked on head c7849f849 (scratch-merged onto main 673f7406c, merge is clean). Windows 11 Pro 26220, Ryzen 7 5700X, WinLibs GCC 15.2. Targeted build only (libs, guest_pthread_timed_locks_tests, guest_pthread_rwlock_tests), not a full build.

  • guest_pthread_timed_locks: 20 of 20 direct runs pass on a loaded host (other builds were running; this is the condition under which it aborted 15 of 20 before), and 20 of 20 again while I ran my own 8-job build next to them. Each run takes 0.5 to 3 s including process start. ctest --repeat until-fail:3: 3 of 3.
  • guest_pthread_rwlock: 3 of 3 with ctest --repeat until-fail:3 and 3 of 3 direct.
  • Negative control: with Mutex.cpp and Rwlock.cpp reverted to main (tests kept), both tests abort with what(): pthread_rwlock_timedrdlock: null abstime; with the files restored both pass.
  • tools/check_conventions.py --base origin/main passes and git diff --check is clean.

The releasing flag is set before the unlock and the lock can only be taken after it, so the check no longer depends on when the main thread is scheduled. That resolves the flake I reported.

Not checked: full build, the non-GPU suite, Linux, title runs.

Written with Claude Code assistance (validation by a subagent, reviewed in this session).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants