Signal handling for non-Python WASM modules by logan-gatlin · Pull Request #6154 · cloudflare/workerd

logan-gatlin · 2026-02-24T16:34:23Z

Continuation of 6098

This adds a shim to WebAssembly.Instantiate and friends which looks for modules that export two i32 globals: __signal_address and __terminated_address. Modules matching one of these signatures will receive a hook, allowing us to read and write to those memory locations, allowing for signal-based communication with modules.

The addition of __terminated_address is required, since we must know when the module is dead so we can stop sending it signals. This is also useful for modules that want to know when they have been terminated, so that they can re-initialize.

ask-bonk

I'm Bonk, and I've done a quick review of your PR.

Summary: Adds a JS shim around WebAssembly.instantiate/Instance to detect WASM modules exporting __signal_address/__terminated_address globals, registering them for signal-based shutdown notification via a new signal-safe AtomicList data structure.

Issues ranked by severity:

[High] Missing compat flag — behavioral change applied unconditionally to all workers. The shim wraps WebAssembly.instantiate's return promise with an extra .then(), which adds a microtask tick before results are delivered. This subtly changes promise scheduling for every worker that uses WebAssembly.instantiate, not just ones with signal globals. workerd has a strong backward-compat commitment; this kind of observable behavioral change must be gated behind a compatibility flag in compatibility-date.capnp.
[High] Missing autogate for staged rollout. Even with a compat flag, this is a risky change (shimming fundamental WebAssembly APIs, new signal-handler code path). It should additionally be gated behind an autogate flag for safe incremental rollout.
[High] No tests. There are no tests for any part of this feature: AtomicList, the JS shim, registerWasmShutdownSignal, or the GC prologue filtering. AtomicList is a custom concurrent data structure used from signal handlers — this absolutely needs unit tests, including tests that verify signal-safety invariants (e.g., that iterate() sees a consistent list while filter() is interrupted).
[Medium] WebAssembly.Instance shim changes behavior when called without new. The original WebAssembly.Instance throws if called as a regular function (not as a constructor). The shim is a plain JS function that internally does new originalInstance(...), so WebAssembly.Instance(module, imports) (without new) would succeed instead of throwing. This is a spec deviation.
[Medium] AtomicList uses GCC __atomic builtins on non-atomic types. Node* head and Node* next are plain pointers, not std::atomic<Node*>. The GCC builtins work in practice, but this is technically undefined behavior per the C++ memory model. Consider using std::atomic<Node*> for head and Node::next — the generated code is identical, and it makes the concurrency intent explicit and portable.
[Low] <atomic> header is included but std::atomic is not used. Either switch to std::atomic (see issue 5) or remove the unused include.
[Low] signalByteOffset and terminatedByteOffset can overlap. The bounds checks don't verify the two offsets point to non-overlapping memory. Overlapping offsets would cause the runtime's own signal write to be misinterpreted as the module's termination flag. A simple KJ_REQUIRE(signalOffset != terminatedOffset) would catch this.

ask-bonk · 2026-02-24T16:47:02Z

src/workerd/io/worker.c++

  setWebAssemblyModuleHasInstance(lock, context);

+  // Shim WebAssembly.instantiate to detect modules exporting "__signal_address".
+  shimWebAssemblyInstantiate(lock, context);


[High] This shim is installed unconditionally for all workers. The .then() wrapper in the instantiate shim adds a microtask tick, which is an observable behavioral change to promise scheduling. Per workerd's backward-compat policy, this should be gated behind a compatibility flag.

Additionally, this is a risky change (shimming fundamental WebAssembly APIs + new signal-handler codepath) and should use an autogate for staged rollout.

@logan-gatlin there is no then() wrapper in the instantiate shim right? This is a complete misclassification I believe.

(re AI comment)

Good job, Bonk! This is exactly the kind of thing we need flagged :-) ... ideally the shim is only installed on workers that actually need it. This will install it on all of them it seems.

This new hook is attached on a per module basis, not a per worker basis. The shim is where we determine which modules are actually listening for the signal, and create the hook. Maybe we need to put it behind a gate?

src/workerd/io/worker.c++

src/workerd/io/wasm-shutdown-signal.h

src/workerd/io/limit-enforcer.h

ask-bonk · 2026-02-24T16:47:03Z

src/workerd/io/wasm-shutdown-signal.h

+
+#include <kj/common.h>
+
+#include <atomic>


[Low] <atomic> is included but std::atomic is not used anywhere in this file. Either switch head/next to std::atomic<Node*> (recommended, see other comment) or remove this include.

This also seems incorrect, as atomic methods are used?

(re AI comment)

ask-bonk · 2026-02-24T16:47:18Z

I'm Bonk, and I've done a quick review of your PR.

Summary: Adds a JS shim around WebAssembly.instantiate/Instance to detect WASM modules exporting __signal_address/__terminated_address globals, registering them for signal-based shutdown notification via a new signal-safe AtomicList data structure.

I found 7 issues and posted them as a batch review on the PR. The highest severity items are:

[High] Missing compat flag — The .then() wrapper adds a microtask tick to WebAssembly.instantiate for all workers, which is an observable behavioral change that needs a compat flag.
[High] Missing autogate — Shimming fundamental WebAssembly APIs with new signal-handler code is risky and should use an autogate for staged rollout.
[High] No tests — No tests for AtomicList, the JS shim, registration, or GC prologue filtering.
[Medium] Instance shim spec deviation — WebAssembly.Instance() without new would succeed instead of throwing (suggestion provided).
[Medium] UB with __atomic builtins on non-atomic types — head/next should be std::atomic<Node*> (suggestion provided).
[Low] Unused <atomic> include.
[Low] No overlap check for signal/terminated offsets (suggestion provided).

github run

codecov-commenter · 2026-02-24T17:08:10Z

Codecov Report

❌ Patch coverage is 85.23810% with 31 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.62%. Comparing base (c0f9d42) to head (334150e).
⚠️ Report is 132 commits behind head on main.

Files with missing lines	Patch %	Lines
src/workerd/io/wasm-shutdown-signal-test.c++	77.92%	0 Missing and 17 partials ⚠️
src/workerd/io/worker.c++	87.25%	10 Missing and 3 partials ⚠️
src/workerd/io/wasm-shutdown-signal.h	96.77%	0 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6154      +/-   ##
==========================================
- Coverage   70.70%   70.62%   -0.08%     
==========================================
  Files         409      415       +6     
  Lines      109411   109986     +575     
  Branches    18018    18111      +93     
==========================================
+ Hits        77356    77679     +323     
- Misses      21246    21472     +226     
- Partials    10809    10835      +26

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

jasnell · 2026-02-24T22:00:09Z

src/workerd/io/limit-enforcer.h

+    auto* base = static_cast<kj::byte*>(backingStore->Data());
+    uint32_t zero = 0;
+    // The address may not be aligned, so use memcpy instead of writing directly
+    memcpy(base + signalOffset, &zero, sizeof(zero));


We shouldn't be using v8::BackingStore and memcpy directly here. Instead, you should wrap the ArrayBuffer with jsg::JsArrayBuffer, use it's toArrayPtr() and copyFrom(...) API. It's just safer overall.

jasnell · 2026-02-24T22:02:33Z

src/workerd/io/worker.c++

+
+  auto webAssembly =
+      jsg::check(context->Global()->Get(context, jsg::v8StrIntern(isolate, "WebAssembly")))
+          .As<v8::Object>();


Should make use of js.global() and the jsg::JsObject API instead. It'll cut down on boilerplate and make this easier.

jasnell · 2026-02-24T22:03:45Z

src/workerd/io/worker.c++

+  // __registerWasmShutdownSignal(memory: WebAssembly.Memory, signalOffset: number,
+  //                              terminatedOffset: number)
+  auto registerCb = [](const v8::FunctionCallbackInfo<v8::Value>& info) {
+    jsg::Lock::from(info.GetIsolate()).withinHandleScope([&] {


Nit: better to separate these...

auto& js = jsg::Lock::from(info.GetIsolate()); js.withinHandleScope(...);

Easier to read and grep for.

jasnell · 2026-02-24T22:05:47Z

src/workerd/io/worker.c++

+      })) {
+        // Convert KJ exception to a JavaScript Error object
+        auto message = jsg::v8Str(isolate, e.getDescription());
+        isolate->ThrowException(v8::Exception::Error(message));


We have js.exceptionToJs(...) for this.

jasnell · 2026-02-24T22:07:18Z

src/workerd/io/worker.c++

+  //   registerShutdown     - the C++ registration callback
+  //   wa                   - the WebAssembly object
+  auto shimSource = jsg::v8Str(isolate,
+      "(function(originalInstantiate, originalInstance, registerShutdown, wa) {\n"


Rather than dropping this large string in line like this, it would be better to have it either be an embed (like how we do the node and cloudflare built-ins) or a static constant, will make it easier to maintain over time.

guybedford

Initial feedback, please let me know when this is ready for final review.

guybedford · 2026-02-24T22:01:08Z

src/workerd/io/worker.c++

+  //   originalInstance     - the original WebAssembly.Instance constructor
+  //   registerShutdown     - the C++ registration callback
+  //   wa                   - the WebAssembly object
+  auto shimSource = jsg::v8Str(isolate,


Can we not include this as a JS source like we do for normal JS builtins, rather than as a string? Would be a lot better if we can properly lint and track this JS code.

src/workerd/io/worker.c++

guybedford · 2026-02-24T22:03:31Z

src/workerd/io/worker.c++

  setWebAssemblyModuleHasInstance(lock, context);

+  // Shim WebAssembly.instantiate to detect modules exporting "__signal_address".
+  shimWebAssemblyInstantiate(lock, context);


@logan-gatlin there is no then() wrapper in the instantiate shim right? This is a complete misclassification I believe.

(re AI comment)

guybedford · 2026-02-24T22:04:07Z

src/workerd/io/wasm-shutdown-signal.h

+
+#include <kj/common.h>
+
+#include <atomic>


This also seems incorrect, as atomic methods are used?

(re AI comment)

src/workerd/io/wasm-shutdown-signal.h

jasnell · 2026-02-24T22:08:12Z

src/workerd/io/worker.c++

+      "})\n"_kj);
+
+  auto shimFactory = jsg::check(v8::Script::Compile(context, shimSource));
+  auto shimFactoryResult = jsg::check(shimFactory->Run(context));


We have jsg::NonModuleScript for this.

jasnell · 2026-02-24T22:08:29Z

src/workerd/io/worker.c++

+
+  // Call the factory — it mutates `wa` in place.
+  v8::Local<v8::Value> args[] = {originalInstantiate, originalInstance, registerFn, webAssembly};
+  jsg::check(shimFactoryFn->Call(context, context->Global(), 4, args));


Using jsg::JsFunction is easier here.

logan-gatlin added 4 commits February 20, 2026 19:20

wip

fa07d1e

Added import check

ee7abee

Moved signal logic

6a6837b

Fixed memory detection

544048a

logan-gatlin mentioned this pull request Feb 24, 2026

Signal handling for non-Python WASM modules #6098

Closed

ask-bonk bot reviewed Feb 24, 2026

View reviewed changes

logan-gatlin added 2 commits February 24, 2026 21:32

tests

ff73150

Format: trailing commas, alphabetical BUILD order, line-wrap fix

a89bde8

jasnell reviewed Feb 24, 2026

View reviewed changes

guybedford reviewed Feb 24, 2026

View reviewed changes

jasnell reviewed Feb 24, 2026

View reviewed changes

Fix CI

334150e


		#include <kj/common.h>

		#include <atomic>


		#include <kj/common.h>

		#include <atomic>

Comments

Conversation

logan-gatlin commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ask-bonk bot left a comment

Choose a reason for hiding this comment

Uh oh!

ask-bonk bot Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

guybedford Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ask-bonk bot Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

guybedford Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ask-bonk bot commented Feb 24, 2026

Uh oh!

codecov-commenter commented Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

guybedford left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

guybedford Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

guybedford Feb 24, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

logan-gatlin commented Feb 24, 2026 •

edited

Loading

guybedford Feb 24, 2026 •

edited

Loading

guybedford Feb 24, 2026 •

edited

Loading

codecov-commenter commented Feb 24, 2026 •

edited

Loading

guybedford Feb 24, 2026 •

edited

Loading

guybedford Feb 24, 2026 •

edited

Loading