Skip to content

fix(libkernel): wake POSIX sleep for thread cancellation - #6908

Merged
adriadam10 merged 2 commits into
boykopovar:mainfrom
Zinebyte:fix/posix-sleep-cancellation
Oct 19, 2026
Merged

adriadam10 merged 2 commits into
boykopovar:mainfrom
Zinebyte:fix/posix-sleep-cancellation

Conversation

@Zinebyte

@Zinebyte Zinebyte commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

What

A cancellation request sent after a guest enters POSIX sleep(10) cannot wake the host sleep_for; cancellation is delivered only when the ten seconds expire. A thread joining that sleeper remains blocked too.

Register the wait with the existing guest cancellation mechanism and keep the original monotonic deadline across notifications. Destroy the local wait state before delivering cancellation. Normal completion still returns zero, and disabled cancellation leaves the sleep running to its deadline.

This follows FreeBSD 11.4: sleep delegates to interposed nanosleep, whose libthr wrapper enters a cancellable blocking syscall. The change is limited to POSIX sleep; #6658 changes a separate short GPU polling sleep path in Time.cpp. usleep, sceKernelUsleep, nanosleep, _nanosleep, sceKernelNanosleep and sceKernelSleep still check cancellation only before and after their host sleep. Interrupting those waits is follow-up work. The Thread cancellation entry in docs/dev/debt/libkernel.md now records that boundary.

Tested

Earlier native validation at b3268beaa823f7e4bd87d805bb72a4594193afda: Linux x86-64, GCC 13.3.0, normal CMake Release build with pinned submodules, rebuilding the actual libc and libkernel libraries. No GPU or retail title was used.

  • Main d9b696ddbb with the final regression fixture: both blocked cancellation cases fail after the original ten-second sleep, while all six normal, disabled-cancellation and pending-zero-sleep controls pass.
  • With the fix, all eight new CTests and four existing nanosleep/cancellation/recovery tests pass (12/12, 1.19 seconds). Default and APS5_COARSE_TIMED_WAITS=1 environments are registered; on Linux both use its existing condition-variable backend.
  • Both blocked cancellation tests pass 100 consecutive runs each. The test observes the registered cancellation wait before requesting cancellation, uses a three-second observation bound against a ten-second sleep, and joins the thread before reporting failure.
  • Independent adversarial review checked cancellation races, deadline units, wait-state teardown and disabled cancellation, and reran all 12 native tests successfully.
  • Conventions and whitespace checks pass. CTest JSON confirms all eight cases invoke the new executable; the repository's separate registration auditor requires Ninja and does not support this Unix Makefiles build.

This feedback revision updates the technical-debt entry and the description, and refreshes the branch onto main f3263dfbf3565b414800c3e2e0d19fda3aca7577. The POSIX sleep fix and its regression fixture are unchanged from b3268beaa823f7e4bd87d805bb72a4594193afda. The debt wording was checked against the actual CancellableSleep call paths; conventions and whitespace checks pass.

Fresh verification at 571556754e754bf097145c736fa0b5c2116dc717: Linux x86-64, GCC 13.3, Release. An isolated CMake wrapper includes the actual production libc/libkernel CMake files and pinned zstd/ZArchive sources, rebuilding both libraries and four real test executables without reused objects. All 12 focused CTests pass in 2.02 seconds: eight sleep cases plus nanosleep, cancellation and two recovery cases. Dynamic-library resolution confirms that the tests load the newly built scratch libraries. This is a scoped native build, not a full project build.

No Windows or macOS runtime validation. Full hosted runtime builds require collaborator dispatch under the current contribution policy.

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; no silent stubs added
  • No notes, investigation .md files or images added to the repository
  • New third-party code is a submodule built from source (none added)
  • Generic behaviour, not specific to one title
  • Depends on: none
  • Unblocks: none
  • GPUs tested: n/a
  • AI-assisted: yes — Codex. I am responsible for the change.

@github-actions

github-actions Bot commented Oct 16, 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 7135 docs/dev/debt/libkernel.md L154-156

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
docs/dev/debt/libkernel.md
PR 6044 core/libs/prx/libkernel/CMakeLists.txt
PR 6149 docs/dev/debt/libkernel.md
PR 6226 core/libs/prx/libkernel/CMakeLists.txt
PR 6446 docs/dev/debt/libkernel.md
PR 6469 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 6519 docs/dev/debt/libkernel.md
PR 6740 docs/dev/debt/libkernel.md
PR 6936 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 6942 core/libs/prx/libkernel/CMakeLists.txt
PR 6977 core/libs/prx/libkernel/CMakeLists.txt
docs/dev/debt/libkernel.md
PR 7049 docs/dev/debt/libkernel.md
PR 7105 docs/dev/debt/libkernel.md
PR 7149 docs/dev/debt/libkernel.md
PR 7201 core/libs/prx/libkernel/CMakeLists.txt
PR 7509 docs/dev/debt/libkernel.md
PR 7554 docs/dev/debt/libkernel.md

@Dextroid17

Copy link
Copy Markdown
Contributor

Linux run of this head, plus a check of the other sleeps.

Setup: head b3268beaa (merge-base with main d9b696ddb), BUILD_TESTING=ON, Ninja, CachyOS (kernel 7.2.9), i9-11900K, GCC 16.2.1. Tests ran under env -i with only HOME and PATH set. Each arm ran ctest -R '^guest_sleep_cancel_' (8 tests) 3 times.

  • Head: 8 of 8 pass in each of the 3 runs (about 4.1 s per run).
  • Revert arm: with Thread.cpp as on d9b696ddb and the PR's test kept, blocked and blocked_coarse fail 3 of 3 after 10.1 s with sleep stayed blocked after cancellation (GuestSleepCancel.cpp:99). The other 6 pass.
  • Merged with main at e751dd205: clean merge, 8 of 8 pass in each of the 3 runs. tools/check_conventions.py passes. main has since moved to f3263dfbf; it does not touch this PR's files and the PR still merges cleanly.
  • Under load: the merged build ran with ctest -j16 --repeat until-fail:25 --schedule-random, so up to 8 tests ran in parallel, 25 times each (200 runs): 0 failures.

Two things the PR does not cover:

  1. The other sleeps still do not wake. As a probe, I changed the test's sleep_nid_postfix(seconds) locally to usleep_nid_postfix(seconds * 1000000u) and then to sceKernelUsleep_nid_postfix(seconds * 1000000u). In both cases blocked fails after 10.1 s with the same message, and normal passes. These calls, plus nanosleep, _nanosleep, sceKernelNanosleep and sceKernelSleep (read in the code, not run), all go through CancellableSleep in Time/Time.cpp, which checks before and after SleepNanos but not during it. In FreeBSD 11.4 nanosleep itself is the cancellation point (__thr_nanosleep wraps the syscall in _thr_cancel_enter/_thr_cancel_leave), and sleep reaches it through that path. (The description's libthr link, #L264-L275, lands on __thr_clock_nanosleep; it has the same shape.) If the other sleeps are left for a follow-up PR, that is fine, but then:
  2. The debt entry needs an update. docs/dev/debt/libkernel.md (Thread cancellation) still says "A sleep is not interrupted: the thread exits when it ends." After this PR that is no longer true for sleep, and it is still true for the six calls above.

Not checked: Windows and macOS, and PS5 hardware.

Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.

AI-assisted: yes

@Zinebyte
Zinebyte force-pushed the fix/posix-sleep-cancellation branch from b3268be to 5715567 Compare October 17, 2026 00:44
@github-actions github-actions Bot added the docs label Oct 17, 2026
@Dextroid17

Copy link
Copy Markdown
Contributor

Thanks for updating the debt line in 571556754. I re-ran the head, revert and merge checks on the new head (not the under-load run from last time).

Setup: head 571556754 (merge-base with main f3263dfbf), Release, BUILD_TESTING=ON, Ninja, CachyOS (kernel 7.2.9), i9-11900K, GCC 16.2.1. The three checks ran under env -i with only HOME and PATH set. Each ran ctest -R '^guest_sleep_cancel_' (8 tests) 3 times. git range-diff shows your fix commit is unchanged from b3268beaa; the only new change is the debt line.

  • Head: 8 of 8 pass in each of the 3 runs.
  • Fix reverted: with Pthread/Posix/Thread.cpp as on f3263dfbf and your test kept, blocked and blocked_coarse fail 3 of 3 after about 10.05 s with sleep stayed blocked after cancellation (GuestSleepCancel.cpp:99). The other 6 pass.
  • Merged with main at e1ac3300d: clean merge, 8 of 8 pass in each of the 3 runs. tools/check_conventions.py passes.

The new debt line matches what I measured for sleep, usleep and sceKernelUsleep. WaitForSleep waits until the thread's cancelWait is set. Your sleep sets it through ThreadCancel::Wait. usleep and sceKernelUsleep go through CancellableSleep in Time/Time.cpp, which never sets it, so that wait would time out.

So for the probe I changed the test locally: the sleeper calls usleep_nid_postfix(seconds * 1000000u), then sceKernelUsleep_nid_postfix(seconds * 1000000u); the main thread cancels after a fixed 1 s instead of WaitForSleep and sets wasSleeping to true so the later check still passes; ObservationLimit is raised to 15 s. I ran blocked once per call. In both runs the test passes: the thread's result is PTHREAD_CANCELED, and the line after the sleep call never runs. Each run takes 10.00 s, not about 1 s, so the request sent 1 s into the sleep took effect only when the 10 s sleep ended, as the line says.

Two earlier probe runs that kept WaitForSleep are not counted. That wait ran out at 15 s, after the 10 s sleep had already ended, so the cancel came too late to test the line.

Probe edit (usleep; the sceKernelUsleep one differs only in the name)
-static constexpr auto ObservationLimit = std::chrono::seconds(3);
+static constexpr auto ObservationLimit = std::chrono::seconds(15);
+int APS5_VABI usleep_nid_postfix(unsigned int);
-    Require(sleep_nid_postfix(seconds) == 0);
+    Require(usleep_nid_postfix(seconds * 1000000u) == 0);
-        if (!pending) wasSleeping = WaitForSleep(thread);
+        if (!pending) { std::this_thread::sleep_for(std::chrono::seconds(1)); wasSleeping = true; }

Heads-up on #6540: the debt commit now conflicts with #6540 in docs/dev/debt/libkernel.md. This PR changes the Thread cancellation entry and #6540 (head 7777219ee) changes the cpuset_setaffinity and cpuset_getaffinity entries directly below it, so git merge-tree reports them as one conflict region. b3268beaa merged cleanly with #6540, and each PR still merges cleanly with main at e1ac3300d on its own. Whichever lands second will need a small rebase.

Not checked: Windows and macOS, and PS5 hardware. The other four sleeps in the debt line (nanosleep, _nanosleep, sceKernelNanosleep, sceKernelSleep) were read in the code, not run. This PR and #6540 were not built or tested together.

Written with AI assistance (Claude Code). I ran the builds and tests above on my machine.

AI-assisted: yes

@github-actions github-actions Bot added the needs rebase Conflicts with main, rebase needed label Oct 17, 2026
@cadenasynyster

Copy link
Copy Markdown
Contributor

Windows run, since both runs so far are Linux-only. On Windows the two registered environments exercise different TimedWait::Condition backends (default: high-resolution waitable timers; APS5_COARSE_TIMED_WAITS=1: the condition-variable path), so _coarse and default aren't duplicates here.

Windows 11 10.0.26200.9711, Ryzen 7 9800X3D, WinLibs GCC 15.2.0 r7, Release, BUILD_TESTING=ON. Head 5715567 merged onto main b51c65d: Thread.cpp and CMakeLists.txt merge cleanly; the only conflict is docs/dev/debt/libkernel.md (the build doesn't read it).

ctest -R "^guest_sleep_cancel_" -j1, 3 rounds: 8/8 each round.

test default coarse
blocked pass, 0.01-0.08 s pass, 0.01 s
disabled pass, 1.01 s pass, 1.01-1.02 s
normal pass, 1.01 s pass, 1.02 s
pending pass, 0.01 s pass, 0.01 s

Control: same build with only core/libs/prx/libkernel/Pthread/Posix/Thread.cpp restored from main. Both guest_sleep_cancel_blocked and guest_sleep_cancel_blocked_coarse fail after about 12 s (sleep stayed blocked after cancellation, exit 0xc0000409). So the fixture catches the bug on Windows on both backends, and the PR fixes it.

Not checked: macOS, a title, and runs under load.

AI-assisted: yes

@adriadam10
adriadam10 force-pushed the fix/posix-sleep-cancellation branch from 5715567 to f4d68f2 Compare October 19, 2026 11:01
@github-actions github-actions Bot removed the needs rebase Conflicts with main, rebase needed label Oct 19, 2026
@adriadam10
adriadam10 merged commit f436675 into boykopovar:main Oct 19, 2026
10 checks passed
@adriadam10

Copy link
Copy Markdown
Collaborator

Rebased onto current main: the only conflict was the adjacent Thread cancellation line in docs/dev/debt/libkernel.md; kept main's new lines and this PR's sentence about POSIX sleep. Re-checked the ThreadCancel::Wait deadline units, the disabled-cancellation and zero-sleep paths, and that the new tests fail without the change (also confirmed by @Dextroid17 on Linux and @cadenasynyster on Windows). libkernel/libs ctest passes (225/225); merged.

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.

4 participants