Clean up EventTarget and AbortSignal implementation

docs/reference/detail/abort-signal.md

@@ -0,0 +1,128 @@
+# AbortSignal internals: cancellation hooks and the cross-request model
+
+This document describes how `api::AbortSignal` (`src/workerd/api/basics.h`) delivers aborts
+to native (C++) consumers, and which primitive to use when hooking work to a signal. It is
+aimed at runtime code; the JS-visible behavior follows the WHATWG DOM spec.
+
+## State model
+
+An `AbortSignal` is a plain JS-heap object. Its abort state — `maybeAbortException` (a
+`kj::Exception` derived from the abort reason) and `reason` (the JS value) — requires no
+`IoContext` to create or read. Consequently:
+
+- `new AbortController()`, `AbortSignal.abort()`, and objects that allocate signals (e.g.
+  `WritableStream`, `request.signal`) work at global scope, during module evaluation.
+- A single signal may be created in one request, observed in another, and aborted from a
+  third (or from outside any request). JS-visible effects of an abort (state, events) happen
+  synchronously in whichever context triggers it.
+
+What *does* involve I/O — cancelling KJ promises, notifying RPC peers — is handled through
+per-registration cells bound to the registering request, described below.
+
+## Choosing a primitive
+
+| You have...                                                    | Use                                     |
+| -------------------------------------------------------------- | ---------------------------------------- |
+| A `kj::Promise` to cancel on abort                               | `signal->wrap(js, promise)`               |
+| A KJ-side object needing promise wrapping + a cancel callback | `signal->newCanceler(js)` + `AbortableImpl` |
+| A native callback to run on abort, in your request's context  | `signal->addAbortAction(js, fn)`          |
+| A JS-heap reaction (no IoContext involvement)                 | `signal->addAbortAlgorithm(js, fn)`       |
+
+All four are no-ops (or immediate, see below) for `NEVER_ABORTS` signals, and all native
+variants require an active `IoContext` at registration time.
+
+### `wrap(js, promise)`
+
+Returns a promise that rejects with a `kj::Exception` derived from the abort reason when the
+signal aborts. If the signal is already aborted, the returned promise rejects immediately the
+same way — indistinguishable from an abort arriving right after wrapping. Callers that want
+to surface the JS reason with value identity (e.g. `fetch`) should pre-check `getAborted()`
+and use `getReason()` themselves.
+
+### `newCanceler(js)`
+
+Returns `Cancellation{canceler, registration}`:
+
+- `canceler` is a solely-owned `ReleasingCanceler` (`src/workerd/util/canceler.h`): wrap I/O
+  promises through it, register `ReleasingCanceler::Listener`s for cancel callbacks. Its
+  destructor *releases* (never cancels) still-wrapped promises; a `Listener` registered
+  after cancellation fires immediately.
+- `registration` keeps the canceler hooked to the signal. **Destroy the registration before
+  the canceler** (declare it after the canceler member): the signal reaches the canceler by
+  reference, valid only while the registration exists.
+
+### `addAbortAction(js, fn)`
+
+Registers `fn(js, exception)` to run at most once when the signal aborts, always under the
+isolate lock, always in the IoContext current at registration time:
+
+- Abort triggered in that context: `fn` runs synchronously during the abort.
+- Abort triggered elsewhere (another request, or no request): delivery is deferred into the
+  owning context via `IoCrossContextExecutor::tryExecute` and runs on its next turn. The
+  deferred task re-takes the registration slot on arrival, so a consumer that went away in
+  the meantime turns the delivery into a guaranteed no-op.
+- Owning context already destroyed: the delivery is silently dropped — everything the action
+  wanted to touch died with the context.
+
+Dropping the returned handle (safe from any thread) guarantees `fn` never runs again, so
+`fn` may capture plain references whose lifetime the holder ties to the handle. This is also
+the single point that arms the RPC abort subscription for deserialized signals — every
+native registration path funnels through it.
+
+`ExecProcess`'s kill-on-abort is the canonical direct consumer.
+
+### `addAbortAlgorithm(js, fn)`
+
+The DOM spec's "add an algorithm to signal's abort algorithms", for reactions whose state
+lives entirely on the JS heap. Runs under the isolate lock in *whichever* context triggers
+the abort, before the `abort` event is dispatched; never runs for synthetic
+`dispatchEvent('abort')` calls. The handle holds only a `jsg::WeakRef` to the signal and must
+be dropped under the isolate lock. Capture JS-heap objects weakly (`JSG_THIS_WEAK`) unless
+the signal genuinely should keep them alive: algorithms are owned and GC-visited by the
+signal, so a strong capture makes a long-lived signal retain the captured object.
+
+`addEventListener`'s `{signal}` option is the canonical consumer.
+
+## Abort sequence
+
+`triggerAbort` maps 1:1 onto the spec's "signal abort":
+
+1. Record the abort reason (and exception) on this signal.
+2. Record it on every not-yet-aborted dependent signal (`AbortSignal.any()` results), before
+   any events fire anywhere.
+3. Run this signal's abort steps: abort algorithms (FIFO, then emptied) → native
+   registrations (routed per owner as above) → RPC clones (reason serialized once) → fire
+   the `abort` event (listener exceptions are reported, not propagated; `abort()` cannot
+   throw) → drop all listeners.
+4. Run each collected dependent's abort steps, unlinking it from any remaining sources.
+
+## Lifetime and reclamation
+
+Native and RPC registrations are `kj::Arc`'d cells: an immutable
+`IoCrossContextExecutor` plus a mutex-guarded slot holding the context-bound payload. The
+consumer-side RAII handle clears the slot from any thread; because handles are attached to
+request-owned objects (the wrapped promise, the `AbortableImpl`, ...), IoContext teardown
+reclaims payloads automatically without touching the signal. Empty or defunct-context cells
+are swept on the next registration, bounding a long-lived signal's footprint by its live
+registrations (asserted by `crossRequestRegistrationChurn` in
+`src/workerd/api/tests/abortsignal-test.js` via `getNativeRegistrationCountForTest()`).
+
+Cells hold no JS-heap references and need no GC visitation; abort algorithms do, and are
+visited by the signal.
+
+## Signals received over RPC
+
+A deserialized signal's pending abort reason arrives through a mutex-guarded, atomically
+refcounted box (`ExternalPusherImpl::PendingAbortReasonBox`) written by the receiving
+request's RPC machinery and readable from any context — so `getAborted()`/`getReason()`
+answer correctly even for signals that crossed request boundaries before the abort was
+processed. Actually *reacting* to the abort (running `triggerAbort`) requires the
+subscription armed via `subscribeToRpcAbort()`, which happens automatically when an `abort`
+listener, `onabort` handler, or any native registration is added. Only the receiving
+request's context can arm it — the underlying promise belongs to that request — so arming
+requested from any other context is routed there through the signal's
+`IoCrossContextExecutor` and takes effect on the receiving request's next turn. If the
+receiving request is already gone, the routing is dropped: no delivery is possible anymore.
+Polling stays accurate regardless (if the peer could still abort, that request's teardown
+wrote an implicit connection-lost abort into the box), and registrations made after that
+point observe the abort through the already-aborted pre-checks instead.

src/workerd/api/AGENTS.md

@@ -18,6 +18,7 @@ tests/               # JS integration tests (238 entries); each test = .js + .wd
 | Task                           | Files                                                 |
 | ------------------------------ | ----------------------------------------------------- |
 | fetch / Request / Response     | `http.h`, `http.c++`                                  |
+| Event / EventTarget / AbortSignal | `basics.{h,c++}`; subclasses in `events.h`, per-API headers |
 | Headers                        | `headers.h`, `headers.c++`                            |
 | WebSocket                      | `web-socket.h`, `web-socket.c++`                      |
 | Hibernatable WS (DO)           | `hibernatable-web-socket.h`                           |
@@ -40,6 +41,7 @@ tests/               # JS integration tests (238 entries); each test = .js + .wd
 ## CONVENTIONS
 
 - `global-scope.h` forward-declares most API classes; `ServiceWorkerGlobalScope` registers all nested types
+- Event dispatch: `EventTarget::dispatchEventImpl` takes a `DispatchExceptionPolicy` (PROPAGATE for runtime top-level delivery, REPORT for spec surfaces); UA-fired WebSocket/EventSource/MessagePort dispatches use REPORT plus a fail-fast reaction via `DispatchResult::firstException`. Events are untrusted by default; runtime construction sites pass `Trusted::YES`. `on<type>` handler attributes go through `EventTarget::setEventHandlerAttribute` (positioned trampoline listeners)
 - Node.js compat: add C++ class + register in `NODEJS_MODULES(V)` macro in `node/node.h`; experimental modules go in `NODEJS_MODULES_EXPERIMENTAL(V)`
 - URL has dual impl: legacy (`url.h`) vs standard (`url-standard.h`); compat flag selects which
 - Streams has dual impl: internal (`streams/internal.h`) vs standard (`streams/standard.h`)

src/workerd/api/basics-test.c++

@@ -18,81 +18,87 @@
 namespace workerd::api {
 namespace {
 
-jsg::V8System v8System;
+jsg::V8System v8System({"--expose-gc"_kj});
 
 struct BasicsContext: public jsg::Object, public jsg::ContextGlobal {
 
-  bool testNativeListenersWork(jsg::Lock& js) {
-    auto target = js.alloc<api::EventTarget>();
-
-    int called = 0;
-    bool onceCalled = false;
-
-    // Should be invoked multiple times.
-    auto handler = target->newNativeHandler(js, kj::str("foo"),
-        [&called](jsg::Lock& js, jsg::Ref<api::Event> event) { called++; }, false);
-
-    // Should only be invoked once.
-    auto handlerOnce = target->newNativeHandler(
-        js, kj::str("foo"), [&](jsg::Lock& js, jsg::Ref<api::Event> event) {
-      KJ_ASSERT(!onceCalled);
-      onceCalled = true;
-      // Recursively dispatching the event here should not cause this handler to
-      // be invoked again.
-      target->dispatchEventImpl(js, js.alloc<api::Event>(kj::str("foo")));
-    }, true);
-
-    KJ_ASSERT(target->dispatchEventImpl(js, js.alloc<api::Event>(kj::str("foo"))));
-    KJ_ASSERT(target->dispatchEventImpl(js, js.alloc<api::Event>(kj::str("foo"))));
-    KJ_ASSERT(onceCalled);
-    return called == 3;
-  }
+  bool testAbortAlgorithmsRun(jsg::Lock& js) {
+    auto signal = js.alloc<api::AbortSignal>();
+
+    kj::Vector<int> order;
+    auto reg1 = signal->addAbortAlgorithm(js, [&order](jsg::Lock&) { order.add(1); });
+    auto reg2 = signal->addAbortAlgorithm(js, [&order](jsg::Lock&) { order.add(2); });
+    auto reg3 = signal->addAbortAlgorithm(js, [&order](jsg::Lock&) { order.add(3); });
+
+    // Dropping a registration unregisters its algorithm.
+    reg2 = kj::Own<void>();
 
-  bool testCanAddHandlersInHandlers(jsg::Lock& js) {
-    // Exercises a use case that triggered asan failures in earlier implementations.
-    auto target = js.alloc<api::EventTarget>();
-    int toplevelCalls = 0;
-    int otherCalls = 0;
-    kj::Vector<kj::Own<void>> handlers;
+    // A synthetic dispatch of an 'abort' event does not run abort algorithms; only a real
+    // abort does.
+    signal->dispatchEventImpl(js, js.alloc<api::Event>(kj::str("abort")));
+    KJ_ASSERT(order.empty());
 
-    handlers.add(target->newNativeHandler(
-        js, kj::str("foo"), [&](jsg::Lock& js, jsg::Ref<api::Event> event) {
-      toplevelCalls++;
+    signal->triggerAbort(js, kj::none);
+    KJ_ASSERT(order.size() == 2);
+    KJ_ASSERT(order[0] == 1);
+    KJ_ASSERT(order[1] == 3);
+    KJ_ASSERT(signal->getAborted(js));
 
-      for (int i = 0; i < 16; ++i) {
-        handlers.add(target->newNativeHandler(js, kj::str("foo", i),
-            [&](jsg::Lock& js, jsg::Ref<api::Event> event) { otherCalls++; }, false));
-      }
-    }, false));
+    // Algorithms are emptied by the abort; a second trigger is a no-op.
+    signal->triggerAbort(js, kj::none);
+    KJ_ASSERT(order.size() == 2);
+    return true;
+  }
 
-    handlers.add(target->newNativeHandler(js, kj::str("foo"),
-        [&](jsg::Lock& js, jsg::Ref<api::Event> event) { toplevelCalls++; }, false));
+  bool testAbortAlgorithmHandleAfterSignalGone(jsg::Lock& js) {
+    // A registration handle may safely outlive its signal: dropping it afterward is a no-op.
+    kj::Own<void> reg;
+    {
+      auto signal = js.alloc<api::AbortSignal>();
+      reg = signal->addAbortAlgorithm(js, [](jsg::Lock&) {});
+    }
+    js.v8Isolate->RequestGarbageCollectionForTesting(v8::Isolate::kFullGarbageCollection);
+    reg = kj::Own<void>();
+    return true;
+  }
 
-    KJ_ASSERT(target->dispatchEventImpl(js, js.alloc<api::Event>(kj::str("foo"))));
+  bool testAbortAlgorithmAddedWhileAborted(jsg::Lock& js) {
+    // Callers are expected to check getAborted() first; an algorithm registered against an
+    // already-aborted signal never runs (a real abort happens at most once).
+    auto signal = js.alloc<api::AbortSignal>();
+    signal->triggerAbort(js, kj::none);
 
-    KJ_ASSERT(toplevelCalls == 2);
-    KJ_ASSERT(otherCalls == 0);
+    bool called = false;
+    auto reg = signal->addAbortAlgorithm(js, [&called](jsg::Lock&) { called = true; });
+    signal->triggerAbort(js, kj::none);
+    KJ_ASSERT(!called);
     return true;
   }
 
   JSG_RESOURCE_TYPE(BasicsContext) {
-    JSG_METHOD(testNativeListenersWork);
-    JSG_METHOD(testCanAddHandlersInHandlers);
+    JSG_METHOD(testAbortAlgorithmsRun);
+    JSG_METHOD(testAbortAlgorithmHandleAfterSignalGone);
+    JSG_METHOD(testAbortAlgorithmAddedWhileAborted);
   }
 };
 JSG_DECLARE_ISOLATE_TYPE(BasicsIsolate,
     BasicsContext,
     EW_BASICS_ISOLATE_TYPES,
     jsg::TypeWrapperExtension<PromiseWrapper>);
 
-KJ_TEST("EventTarget native listeners work") {
+KJ_TEST("AbortSignal abort algorithms run in order, once, and only for real aborts") {
+  jsg::test::Evaluator<BasicsContext, BasicsIsolate, CompatibilityFlags::Reader> e(v8System);
+  e.expectEval("testAbortAlgorithmsRun()", "boolean", "true");
+}
+
+KJ_TEST("AbortSignal abort algorithm handles are safe after the signal is gone") {
   jsg::test::Evaluator<BasicsContext, BasicsIsolate, CompatibilityFlags::Reader> e(v8System);
-  e.expectEval("testNativeListenersWork()", "boolean", "true");
+  e.expectEval("testAbortAlgorithmHandleAfterSignalGone()", "boolean", "true");
 }
 
-KJ_TEST("EventTarget can add handlers in handlers") {
+KJ_TEST("AbortSignal abort algorithms registered after abort never run") {
   jsg::test::Evaluator<BasicsContext, BasicsIsolate, CompatibilityFlags::Reader> e(v8System);
-  e.expectEval("testCanAddHandlersInHandlers()", "boolean", "true");
+  e.expectEval("testAbortAlgorithmAddedWhileAborted()", "boolean", "true");
 }
 
 }  // namespace