Skip to content

fix(workers): release thread-local event names after VM teardown - #153

Merged
steipete merged 2 commits into
mainfrom
claude/worker-thread-cache-release
Oct 9, 2026
Merged

steipete merged 2 commits into
mainfrom
claude/worker-thread-cache-release

Conversation

@steipete

@steipete steipete commented Oct 9, 2026 •

Copy link
Copy Markdown

Repeated Worker creation retained each thread's EventNames cache because C++ TLS destructors are disabled. This ports oven-sh#38164 and releases the cache explicitly on the exiting Worker thread, after VM destruction and before the thread's atom table disappears. Live-thread caching and main-thread exit behavior are unchanged. The upstream PR remains open; its EventNames release is credited to @robobun.

The branch is based on merged #152. An addon can supply the first external string adopted as an event-name atom; when the cache releases it after env cleanup, #152's storage-owned finalizer invokes the addon with env == nullptr. The new regression keeps the matching literal out of the Worker module, verifies no early callback or copy, and requires exactly one null-env callback on the creating thread with the addon's fd closed. Both termination routes fail before this change and pass after it. The previously aborting reproducer now passes Linux ASAN and macOS without any further lifetime-policy or ordering change.

The separate parser leak is fixed in openclaw/WebKit#28 (development) and openclaw/WebKit#29 (release). DefineFieldNode owns an Identifier but previously skipped destruction during arena teardown; those changes register its destructor. Complete leak qualification uses both fixes on the pinned engine source.

Verification Result
Baseline no-addon LSan, 10 / 100 / 1,000 Workers 4,368 / 38,928 / 384,259 bytes; 128 / 1,118 / 11,011 allocations
Candidate, 1,000 Workers, terminate and self-exit Both exit 0 with the inherited console suppression only
Precisely retained console remainder 72 bytes / two allocations per global; 72,072 / 2,002 at 1,000 Workers; tracked in oven-sh#38395
Sanitizer calibration Deliberate 12,345-byte leak detected; deliberate UAF detected
Addon finalizer/fd fixture, Linux ASAN and macOS 8/8 callbacks, zero fds, creating-thread callbacks on both routes
Full N-API suites, Linux and macOS 264 pass, zero failures on each platform
ASAN shutdown regression Five failures before, five passes after
Native parser regression Refcount 1→2→2 before, 1→2→1 after; native test passes
5,000-cycle OS RSS growth, Worker 100→5,000 Linux 9,408,512→6,930,432 bytes; macOS 16,154,624→11,173,888 bytes

Loaded-Worker timing uses eight alternating blocks of 30 Workers per arm and exit route. Median/p95 milliseconds: macOS terminate 6.060/7.516→5.831/6.834, self-exit 6.047/6.778→5.787/6.515; Linux terminate 28.931/31.672→29.431/32.208, self-exit 28.871/31.411→28.497/32.529. Linux results are mixed: paired-block uncertainty includes zero for both routes. This does not establish timing equivalence or a speedup; the small positive shifts are retained, with no discarded or repeated run and no new tolerance.

P2 review is scoped-clean. Exact-head Linux/macOS native CI and Rust CI are green at 38de3d6e68ed3b767575b0de1be7846ccd62d9e5; lint, source checks, and formatting are green too. Sanitizer allocator exports were changed only in verification relinks, with no production linker-map changes or new suppressions.

Integration ordering: #152 must precede this change; it is already merged. A release claiming the complete leak fix must also use a rebuilt engine containing the parser fix. WebKit #28 and #29 target separate branches and have no serial dependency on each other. These PRs remain unmerged; engine publication, Bun repinning, and release are separate follow-up actions.

Ports oven-sh#38164. Explicitly release the Worker-owned cache while the thread atom table remains alive; preserve live-thread caching and main-thread exit behavior.
@steipete
steipete force-pushed the claude/worker-thread-cache-release branch from 89ef517 to 38de3d6 Compare October 9, 2026 20:10
@steipete
steipete marked this pull request as ready for review October 9, 2026 20:45
@steipete
steipete merged commit b549980 into main Oct 9, 2026
10 checks passed
@steipete
steipete deleted the claude/worker-thread-cache-release branch October 9, 2026 20:45
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