Add: cooperative init cancellation and retryable resource cleanup journal - #1673
Merged
Merged
Conversation
📝 WalkthroughWalkthroughThe worker lifecycle now supports deterministic shared-memory naming, cooperative initialization cancellation, and retryable cleanup journaling. Tests add a reusable fake-chip harness and cover hierarchical startup, teardown, registration rollback, slot reuse, and callable eligibility. ChangesWorker lifecycle
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Caller
participant WorkerClose as Worker.close
participant WorkerInit as Worker.init
participant CleanupJournal
Caller->>WorkerClose: close during initialization
WorkerClose->>WorkerInit: request cancellation and wake waiters
WorkerInit->>CleanupJournal: record unreaped child cleanup
WorkerInit-->>WorkerClose: finish initialization as CLOSED
WorkerClose->>CleanupJournal: retry journaled cleanup
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ChaoWao
changed the title
ChaoWao
force-pushed
the
p0.2b1-fake-chip-harness
branch
2 times, most recently
from
August 4, 2026 04:25
e5f815d to
796efd4
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@python/simpler/worker.py`:
- Around line 6768-6783: Update both teardown paths in python/simpler/worker.py
at lines 6768-6783 and 9075-9077: transfer every unreaped PID/SHM pair into
CleanupJournal after each attempt, rather than excluding killed or timed-out
children. Remove or retire the transferred entries from the active resource
lists so successful journal retries clear the residual inventory; update the
logic around _journal_child_survivors() and _reap_child_groups() accordingly.
- Around line 6119-6121: Update the SHM token initialization in the worker
startup flow so recursive startup preserves the inherited root token: generate a
new UUID only when _startup_deadline is None, or ensure _is_startup_root is
updated before the condition runs. Keep the existing token for preconstructed
inner workers initialized with a startup deadline, so descendants remain in the
root namespace.
- Around line 3029-3035: Update _cleanup_child() so failures from
SharedMemory.close() and unlink() are not silently swallowed: still attempt both
operations, propagate any close failure and any unlink OSError other than
FileNotFoundError, and treat only FileNotFoundError from unlink() as successful
already-reclaimed cleanup so CleanupJournal.drive() retains entries for genuine
failures.
In `@tests/ut/py/test_worker/test_p0_2c_cancellation_journal.py`:
- Around line 138-143: Update the test around _hard_timeout and w.close so
close() runs in a separate thread; wait until w._cancel_token is latched,
release the paused initializer via release.set(), then join the closer and
assert it completed without error and that w._lifecycle is CLOSED.
In `@tests/ut/py/test_worker/test_startup_readiness.py`:
- Around line 659-662: Update tests/ut/py/test_worker/test_startup_readiness.py
lines 659-662 and 1372-1376: execute Worker.close() in a separate thread, wait
for cancellation to latch, release the paused initialization
(_start_hierarchical at lines 659-662; ChipWorker.init() at lines 1372-1376),
then join both close and init threads. Assert the close result and
initialization-owner cancellation result only after both threads complete.
- Around line 1492-1506: Update Worker.register() to call
_eligible_target_need() before installing any post-init registration, rejecting
a ChipCallable on a chipless L3 with the expected RuntimeError instead of
creating an inert handle. Remove the strict xfail marker from
test_post_init_chip_callable_on_chipless_l3_rejected so the test runs as a
required passing case.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 29d76f1b-2aa6-428c-b216-b900ceb52aea
📒 Files selected for processing (8)
python/simpler/worker.pytests/ut/py/test_worker/__init__.pytests/ut/py/test_worker/_harness.pytests/ut/py/test_worker/test_host_worker.pytests/ut/py/test_worker/test_l3_l2_orch_comm.pytests/ut/py/test_worker/test_l4_recursive.pytests/ut/py/test_worker/test_p0_2c_cancellation_journal.pytests/ut/py/test_worker/test_startup_readiness.py
ChaoWao
force-pushed
the
p0.2b1-fake-chip-harness
branch
4 times, most recently
from
August 4, 2026 06:51
16eda67 to
f5e1915
Compare
…rnal - close() during INITIALIZING cooperatively cancels in-flight init via latching cancel token and ultimately reaches CLOSED; same-thread close-during-init is rejected - The cancel wait is bounded by _CLOSE_CANCEL_UNWIND_TIMEOUT_S: the token is only observed at cooperative points, so an init blocked in a native segment (ChipWorker.init, the fork phase) reaches none of them and close() raises instead of blocking forever - Root cancellation raises InitCancelled (a RuntimeError), catchable by ordinary handlers; _StartupCancelled stays the signal-delivered BaseException of the forked-child path - CleanupJournal class with post-success deletion, idempotent drive(), and error accumulation shared by both teardown paths - Journal driven before teardown in _teardown_ready_tree; entries that fail stay for retry on next close() - _cleanup_child() propagates non-idempotent shared-memory cleanup failures so the journal retains entries for genuine errors - Startup rollback registers unreaped children in journal instead of unconditionally clearing pid/shm lists - Deterministic shm names from a per-Worker uuid token; a fixed name makes a segment left by a crashed prior run fail loudly on create rather than be silently bypassed. Zero wire delta - close() prior-done replay and residual-synthesis checks are journal-aware (non-empty journal retryable; empty + live = poison) - _has_live_resources() includes journal entries - 18 new tests; 2 old fail-fast tests updated to cooperative cancel
ChaoWao
force-pushed
the
p0.2b1-fake-chip-harness
branch
from
August 4, 2026 08:08
f5e1915 to
3a6ed5d
Compare
Collaborator
Author
|
/run-cpu |
|
❌ /run-cpu lane finished with failure — https://github.com/hw-native-sys/simpler/actions/runs/30892109556
|
|
✅ /run-cpu lane passed — https://github.com/hw-native-sys/simpler/actions/runs/30892109556
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
close() during INITIALIZING now cooperatively cancels the in-flight init
and ultimately reaches CLOSED, instead of failing fast. A latching cancel
token is observed at cooperative points (the child-readiness poll and the
READY commit gate); native segments run to completion uninterrupted.
Because an init blocked inside a native segment reaches no cooperative
point, the wait is bounded by
_CLOSE_CANCEL_UNWIND_TIMEOUT_S— close()raises rather than blocking forever. CLOSED is absorbing: a late init
rollback cannot resurrect FAILED.
Root cancellation raises
InitCancelled, an ordinaryRuntimeError, sothe init-owner thread can catch it with
except Exception._StartupCancelledremains the signal-deliveredBaseExceptionused onlyon the forked-child SIGTERM path.
A CleanupJournal tracks resources whose native free can fail. Entries are
removed only after successful cleanup (post-success), and a subsequent
close() retries any that remain. The journal is shared by both teardown
paths (_teardown_ready_tree and _abort_hierarchical). Unreaped children
from startup rollback are registered in the journal rather than
unconditionally discarded.
Mailbox shm names are now deterministic —
sp-<token[:8]>-<suffix>, whereeach Worker generates its own token. Because the name is fixed rather than
random, a segment left behind by a crashed prior run surfaces as a
FileExistsErroron create instead of being silently bypassed. Zero wiredelta.
close() replay and residual-synthesis checks are journal-aware: a
non-empty journal permits retry; an empty journal with live resources is
poison.
Changes
InitCancelledexception type, CleanupJournal class, journal integration in both
teardown paths, deterministic shm naming, journal-aware close()
terminal checks
uncooperative-init bound, CLOSED absorption, shm naming, and
retry-on-close
Not in scope
Deterministic naming is a precondition for prefix-based orphan
recovery, not the recovery itself: nothing in this PR enumerates
/dev/shm. Reclaiming segments left by a hard-killed process tree isfollow-up work.
Verification
tests/ut/py/test_worker/: 534 passed, 3 skippedtests/ut/py: 1029 passed, 14 skipped (5TestTorchInteropfailures are a local missing-
torchartifact, green in CI)test_uncooperative_init_bounds_the_close_waitconfirmed to failpre-fix (hangs to the test wall budget) and pass post-fix