Skip to content

fix(libSceJobManager): keep persistent workers spinning without the shared lock - #13999

Merged
adriadam10 merged 2 commits into
boykopovar:mainfrom
bipinkrish:fix/jobmanager-worker-spin
Nov 5, 2026
Merged

adriadam10 merged 2 commits into
boykopovar:mainfrom
bipinkrish:fix/jobmanager-worker-spin

Conversation

@bipinkrish

@bipinkrish bipinkrish commented Oct 31, 2026 •

Copy link
Copy Markdown
Contributor

What

Persistent workers repeatedly reacquire the shared manager mutex while idle. On the Windows host reported in #13026, one worker can monopolize that mutex and leave the other persistent workers blocked.

Keep persistent workers spinning on workEpoch outside the mutex until dispatch, shutdown, or a persistent-count change advances the epoch. Preserve the bounded timer-spin path and the existing sleep and wake-token accounting.

The overlap warning for #10249 was checked manually: its JobManager hunk changes Sequence::SubmitCompute, while this PR changes WorkerEntry; neither depends on the other.

Tested

  • Previously reported on the original PR head: Linux Release JobManager tests passed in three full rounds, once pinned to two CPUs, and worker_count passed ten direct runs. Hosted Windows CI passed.
  • The original report's idle Windows starvation was not reproduced on Linux. A rerun on the affected Windows host remains useful.
  • Review validation on Ubuntu 24.04.4 x86-64, GCC 13, Release, after merging main at e67cc81869: rebuilt the five JobManager test targets; ctest --test-dir build -R '^guest_job_manager_' --output-on-failure passed 29/29. The tests ran outside the Codex sandbox because its syscall filter denies the checked guest-memory writes.
  • git diff --check and convention/template checks passed against current upstream main. The required --base origin/main check also passed with eight pre-existing debt-length warnings from the fork's older base.

Checklist

…hared lock

The keep-alive branch every persistent worker takes on an idle manager
dropped and retook manager.lock around each spin window, so with several
workerCount threads the releaser retook the lock before a queued waiter
woke and the other workers starved inside lock acquisition. Burn the
idle window on the workEpoch snapshot without the lock and relock only
when dispatches, the persistent count or shutdown changed it, as
WakeWorkers, StopManager and setPersistentThreadCount bump it.

@MarcZX10 MarcZX10 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed — looks good

At head f0c452699298fa3c2882e820cec591e463b65c1c against base 73f4f7e06e4f210a8c6570e335fd654dc0a86134, the persistent-worker idle loop now waits on the atomic epoch without holding the manager lock; dispatch, shutdown, and persistent-count changes advance that epoch so the worker can reacquire the lock and recheck state (idle loop, dispatch wake, shutdown and persistent-count updates). git diff --check, conventions/template validation, and the current-base merge preview passed; hosted Windows passed. The Linux test and Vulkan results are author-reported, Linux CI was skipped, and I did not build or run tests locally.

@bipinkrish bipinkrish closed this Oct 31, 2026
@bipinkrish

Copy link
Copy Markdown
Contributor Author

Sorry miss-click

@bipinkrish bipinkrish reopened this Oct 31, 2026
@github-actions github-actions Bot added the prx label Oct 31, 2026
@github-actions

Copy link
Copy Markdown

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 10249 core/libs/prx/libSceJobManager/Export.cpp

@github-actions github-actions Bot added the fix error Closes an issue (Fixes #N) label Oct 31, 2026
@adriadam10 adriadam10 self-assigned this Nov 5, 2026
@adriadam10
adriadam10 merged commit 269dae4 into boykopovar:main Nov 5, 2026
12 of 13 checks passed
@adriadam10

Copy link
Copy Markdown
Collaborator

Checked that every path that makes work runnable (runQueues.Push via WakeWorkers, StopManager, setPersistentThreadCount) bumps workEpoch, so persistent workers spinning outside the lock cannot miss a dispatch, shutdown or count change; the timed-spin path is unchanged. Windows and conventions green; merged.

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

Labels

fix error Closes an issue (Fixes #N) prx

Projects

None yet

Development

Successfully merging this pull request may close these issues.

libSceJobManager: persistent workers starve on the shared lock, so guest_job_manager_worker_count fails on an idle host

3 participants