Skip to content

Make teardown reclamation CI wait for a blocked worker#889

Closed
brandonpayton wants to merge 1 commit into
mainfrom
fix/teardown-reclaim-readiness
Closed

Make teardown reclamation CI wait for a blocked worker#889
brandonpayton wants to merge 1 commit into
mainfrom
fix/teardown-reclaim-readiness

Conversation

@brandonpayton

Copy link
Copy Markdown
Member

Summary

The teardown reclamation test used onStarted plus a fixed 250 ms delay as a proxy for the guest reaching nanosleep. onStarted fires when the process worker launches, so under CI contention destroy() could run while the channel was still idle. In that state the teardown implementation correctly has no blocked channel to wake, and the test falsely reports that no cooperative exit occurred.

Subscribe to the kernel worker syscall trace before spawning the fixture and wait until its nanosleep syscall is observed. The trace is drained only after the kernel worker has handled the syscall, making the blocked-channel precondition explicit. Cleanup now also unsubscribes and destroys the host on assertion failures.

This is a test-readiness correction only. Runtime behavior, browser/Node host behavior, ABI, package artifacts, and documentation are unchanged.

Evidence

  • Baseline fixed-delay test under 16 concurrent focused runs: 13 passed, 3 failed at the cooperative-exit assertion.
  • Updated test under the same 16-run contention harness: 16 passed, 0 failed.
  • Focused Vitest test: 1 passed.
  • npm run test:teardown:engines: 3/3 passed on Node/V8 and 3/3 passed on Bun/JavaScriptCore.
  • Host typecheck passed.
  • Prettier check and git diff --check passed.
  • CI-shaped Vitest run after building wasm32 fixtures: 114 test files passed, 860 tests passed, 2 expected failures, and 34 skipped. The changed teardown test passed. The command exited nonzero only because the isolated worktree did not have the generated wasm64 hello64.wasm fixture, leaving two tests in wasm64.test.ts unable to start; CI will supply the complete prepared toolchain/workspace.

No conformance suites were run because this PR changes test synchronization only, not syscall, process, memory, libc, or runtime semantics.

@brandonpayton

Copy link
Copy Markdown
Member Author

Additional hosted evidence: naming-only PR #891 reproduced the exact precondition race twice in full Vitest (teardown-reclaim.test.ts:71, missing cooperative exit), while every unrelated suite passed. #889's own hosted Vitest and full test gate are green. I have temporarily stacked #891 on this branch so its one-commit rename can be validated without folding the readiness fix into that PR; after #889 merges, #891 can retarget main unchanged.

@brandonpayton

Copy link
Copy Markdown
Member Author

Merged PR #907 supersedes this work through the strengthened serialized-host CI fix now on main at 149fb3f98. This head was not merged verbatim; the merged fix covers the same teardown race in the combined runtime state.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant