Skip to content

Make __instance_signal and __instance_terminated fully indepedent #6270

Open
logan-gatlin wants to merge 4 commits intomainfrom
logan/instance-lifetime-tracking
Open

Make __instance_signal and __instance_terminated fully indepedent #6270
logan-gatlin wants to merge 4 commits intomainfrom
logan/instance-lifetime-tracking

Conversation

@logan-gatlin
Copy link
Contributor

@logan-gatlin logan-gatlin commented Mar 6, 2026

Currently, __instance_signal is ignored if __instance_terminated is not present. This is because registering a signal creates a strong reference to the instance's memory, and we need some indication of when it is safe to free it. Keeping a weak reference to the memory is not an option, since weak references are not safe to use in a signal handler.
The new solution is to keep a weak reference to the instance alongside our strong reference to its memory. Then in the GC prologue, in addition to dropping references to memories that have __instance_terminated set, we will also drop memories when our instance weakref is now dead. WASM memories have independent lifetimes from the instance they were spawned from. This is because the instance that created a memory may die while another instance that still lives has imported its memory. Therefore, our strong reference to the memory will not prevent the instance from being GC'd.

logan-gatlin and others added 3 commits March 6, 2026 17:41
Co-authored-by: logan-gatlin <logan-gatlin@users.noreply.github.com>
@ask-bonk
Copy link
Contributor

ask-bonk bot commented Mar 6, 2026

The old code had kj::mv(signalOffset) which was unnecessary for a kj::Maybe<uint32_t> (trivially copyable). The new code just copies both, which is fine.

I've done a thorough review of all the changes. The code is well-structured, safe, and properly tested. I don't see any actionable issues — no logic bugs, no safety issues, no missing compat flags (since it's gated by an existing autogate), and no backward compatibility concerns.

LGTM

github run

@codspeed-hq
Copy link

codspeed-hq bot commented Mar 6, 2026

Merging this PR will not alter performance

✅ 70 untouched benchmarks
⏩ 129 skipped benchmarks1


Comparing logan/instance-lifetime-tracking (314d19b) with main (2536901)

Open in CodSpeed

Footnotes

  1. 129 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@logan-gatlin logan-gatlin marked this pull request as ready for review March 9, 2026 18:47
@logan-gatlin logan-gatlin requested review from a team as code owners March 9, 2026 18:47
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