Skip to content

host: make pthread launch publication atomic - #892

Open
brandonpayton wants to merge 3 commits into
mainfrom
fix/thread-ctor-clone-teardown
Open

host: make pthread launch publication atomic#892
brandonpayton wants to merge 3 commits into
mainfrom
fix/thread-ctor-clone-teardown

Conversation

@brandonpayton

@brandonpayton brandonpayton commented Jul 11, 2026

Copy link
Copy Markdown
Member

Purpose

Make pthread creation truthful and atomic across Node and browser hosts, including when a fork child starts a new thread from an atfork callback.

This unblocks Tcl's normal notifier-thread restart after fork and protects every guest that creates pthreads after process transitions. It is a shared host-runtime fix, not a Tcl compatibility path.

Root cause

Two host defects combined in the failing path:

  1. patchWasmForThread() could infer __wasm_call_ctors from a debug name-section entry when no semantic linker/export evidence existed. Neutralizing that unrelated function produced an invalid thread module, so the Worker failed before running guest code.
  2. clone published success and CLONE_PARENT_SETTID before the backing Worker had instantiated and initialized TLS, stack, and channel state. A setup failure could therefore leave a provisional kernel thread, channel, control slot, or Worker behind after the guest had observed success. Late cleanup could also collide with reused TIDs or channels after exec.

Tcl exposes this normally: its fork-child atfork callback restarts the notifier pthread. The failure belongs to Kandelo's shared thread lifecycle rather than the package.

Fix

  • Recognize __wasm_call_ctors only from semantic export/linker-wrapper evidence, validate its () -> () signature, and never treat debug names as rewrite authority.
  • Require every clone host callback to return a ready-state handle with idempotent start() and prepublication abort() operations.
  • Validate parent-TID writes before provisional allocation and again before publication, preserving valid unaligned pointers and returning EFAULT for invalid ones.
  • Make CH_COMPLETE the publication boundary: all fallible setup and rollback precede it; thread_start follows it and a postpublication start failure becomes ordinary thread death rather than a second clone completion.
  • Tie thread, channel, clear-TID, allocator, and exit-coordinator cleanup to the exact process generation and memory so stale teardown cannot mutate a reused exec generation.
  • Reject malformed first readiness messages, ignore exact duplicates, and settle pending launches when exec, exit, or termination retires a process generation.
  • Apply the same lifecycle and failure semantics to Node and browser worker entries, the custom PlatformIO harness, and the MariaDB package test runner.
  • Document the clone/thread lifecycle and add focused Node and Chromium regression coverage.

Validation

All commands ran through scripts/dev-shell.sh on current ABI 18 main.

  • Host declaration build/typecheck: passed.
  • Focused Vitest across constructor patching, lifecycle rollback, coordinator reuse, browser adapter parity, and full fork-instrument coverage: 89 passed, 2 expected failures, 8 documented skips. P-10 fork-child pthread creation/join passed through both the default and custom PlatformIO paths.
  • Independent rereview reran 48 focused tests on f678e098e: all passed. A separate test-gap audit reran 27 lifecycle/patch tests: all passed. Both reviews accepted the head with no blocking finding.
  • Full CI-shaped Vitest exercised 102 passing files (799 passed, 2 expected failures, 107 skipped). Its only two failures were the absent local wasm64/hello64.wasm fixture; after building the wasm64 musl sysroot and program fixtures, the exact wasm64.test.ts suite passed 3/3.
  • Chromium fork-child-thread.spec.ts: passed with the locally built P-10 fixture and local kernel.
  • ABI snapshot/header/TypeScript binding check: passed unchanged; no ABI bump is required.
  • git diff --check: passed; worktree clean.

The initial browser attempt stopped during Vite dependency scanning because ABI 18 node.wasm and node-vfs.vfs.zst were absent. After resolving those declared demo prerequisites, the exact Chromium test passed. Those registry artifacts are test-runner setup only and are not Homebrew bottle evidence.

CI should run the complete kernel, fork-instrument, Vitest, browser, libc, POSIX, and Sortix gates on the pushed head. No package archive, Homebrew bottle, VFS release artifact, or ABI contract changes in this PR.

Require every clone host callback to return an abortable ready-state handle. Validate parent-TID writes before provisional allocation, tie cleanup to process generations and memory identity, and keep all rollback before CH_COMPLETE. Retire pending launches during exec/exit on both Node and browser hosts, preserve custom PlatformIO and MariaDB runners, and cover invalid pointers, stale generations, publication failure, reused coordinators, and the P-10 custom-host path.
@brandonpayton brandonpayton changed the title host: preserve fork-child pthread launches host: make pthread launch publication atomic Jul 12, 2026
@brandonpayton

Copy link
Copy Markdown
Member Author

CI update for f678e098e: the Vitest job ran 117 passing files / 884 passing tests and failed only host/test/teardown-reclaim.test.ts:71 because the cooperative exit had not been observed before the assertion. This is the exact pre-#889 teardown-readiness race also seen on unrelated #884/#888 heads; #889 fixes it by waiting for the blocked worker and is green at bcd704a6a. I am not rerunning the flaky pre-fix test to manufacture a green check. After #889 lands, this branch should be rebased and the complete gate rerun. The new multi-worker publication/rollback tests passed in this CI run.

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