diff --git a/docs/reference/detail/abort-signal.md b/docs/reference/detail/abort-signal.md new file mode 100644 index 00000000000..bd3e2f86750 --- /dev/null +++ b/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. diff --git a/src/workerd/api/AGENTS.md b/src/workerd/api/AGENTS.md index 53922c6ff96..a6aa721e170 100644 --- a/src/workerd/api/AGENTS.md +++ b/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` 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`) diff --git a/src/workerd/api/basics-test.c++ b/src/workerd/api/basics-test.c++ index 9e242ca7907..fcab033fb5a 100644 --- a/src/workerd/api/basics-test.c++ +++ b/src/workerd/api/basics-test.c++ @@ -18,66 +18,67 @@ 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(); - - 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 event) { called++; }, false); - - // Should only be invoked once. - auto handlerOnce = target->newNativeHandler( - js, kj::str("foo"), [&](jsg::Lock& js, jsg::Ref 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(kj::str("foo"))); - }, true); - - KJ_ASSERT(target->dispatchEventImpl(js, js.alloc(kj::str("foo")))); - KJ_ASSERT(target->dispatchEventImpl(js, js.alloc(kj::str("foo")))); - KJ_ASSERT(onceCalled); - return called == 3; - } + bool testAbortAlgorithmsRun(jsg::Lock& js) { + auto signal = js.alloc(); + + kj::Vector 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(); - bool testCanAddHandlersInHandlers(jsg::Lock& js) { - // Exercises a use case that triggered asan failures in earlier implementations. - auto target = js.alloc(); - int toplevelCalls = 0; - int otherCalls = 0; - kj::Vector> handlers; + // A synthetic dispatch of an 'abort' event does not run abort algorithms; only a real + // abort does. + signal->dispatchEventImpl(js, js.alloc(kj::str("abort"))); + KJ_ASSERT(order.empty()); - handlers.add(target->newNativeHandler( - js, kj::str("foo"), [&](jsg::Lock& js, jsg::Ref 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 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 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 reg; + { + auto signal = js.alloc(); + reg = signal->addAbortAlgorithm(js, [](jsg::Lock&) {}); + } + js.v8Isolate->RequestGarbageCollectionForTesting(v8::Isolate::kFullGarbageCollection); + reg = kj::Own(); + return true; + } - KJ_ASSERT(target->dispatchEventImpl(js, js.alloc(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(); + 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, @@ -85,14 +86,19 @@ JSG_DECLARE_ISOLATE_TYPE(BasicsIsolate, EW_BASICS_ISOLATE_TYPES, jsg::TypeWrapperExtension); -KJ_TEST("EventTarget native listeners work") { +KJ_TEST("AbortSignal abort algorithms run in order, once, and only for real aborts") { + jsg::test::Evaluator e(v8System); + e.expectEval("testAbortAlgorithmsRun()", "boolean", "true"); +} + +KJ_TEST("AbortSignal abort algorithm handles are safe after the signal is gone") { jsg::test::Evaluator 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 e(v8System); - e.expectEval("testCanAddHandlersInHandlers()", "boolean", "true"); + e.expectEval("testAbortAlgorithmAddedWhileAborted()", "boolean", "true"); } } // namespace diff --git a/src/workerd/api/basics.c++ b/src/workerd/api/basics.c++ index b592e3b3ed5..24520b8ea1f 100644 --- a/src/workerd/api/basics.c++ +++ b/src/workerd/api/basics.c++ @@ -34,97 +34,14 @@ constexpr bool isSpecialEventType(kj::StringPtr type) { } } // namespace -EventTarget::NativeHandler::NativeHandler( - jsg::Lock& js, EventTarget& target, kj::String type, jsg::Function func, bool once) - : type(kj::mv(type)), - state(State{ - .target = target, - .func = kj::mv(func), - }), - once(once) { - target.addNativeListener(js, *this); -} - -EventTarget::NativeHandler::~NativeHandler() noexcept(false) { - detach(); -} - -void EventTarget::NativeHandler::operator()(jsg::Lock& js, jsg::Ref event) { - KJ_IF_SOME(s, state) { - if (once) { - auto fn = kj::mv(s.func); - detach(); - fn(js, kj::mv(event)); - // Note that the function may have detached itself and caused the NativeHandler - // to be destroyed. Let's be careful not to touch it after this point. - } else { - s.func(js, kj::mv(event)); - } - return; - } -} - -uint EventTarget::NativeHandler::hashCode() const { - return kj::hashCode(this); -} - -void EventTarget::NativeHandler::visitForGc(jsg::GcVisitor& visitor) { - KJ_IF_SOME(s, state) { - visitor.visit(s.func); - } -} - -void EventTarget::NativeHandler::detach() { - KJ_IF_SOME(s, state) { - s.target.removeNativeListener(*this); - state = kj::none; - } -} - -kj::Own EventTarget::newNativeHandler( - jsg::Lock& js, kj::String type, jsg::Function)> func, bool once) { - return kj::heap(js, *this, kj::mv(type), kj::mv(func), once); -} - -const EventTarget::EventHandler::Handler& EventTarget::EventHandlerHashCallbacks::keyForRow( +const jsg::HashableV8Ref& EventTarget::EventHandlerHashCallbacks::keyForRow( const kj::Own& row) const { - // The key for each EventHandler struct is the handler, which is a kj::OneOf - // of either a JavaScriptHandler or NativeHandler. - return row->handler; + return row->identity; } bool EventTarget::EventHandlerHashCallbacks::matches( const kj::Own& a, const jsg::HashableV8Ref& b) const { - KJ_IF_SOME(jsA, a->handler.tryGet()) { - return jsA.identity == b; - } - return false; -} - -bool EventTarget::EventHandlerHashCallbacks::matches( - const kj::Own& a, const NativeHandler& b) const { - KJ_IF_SOME(ref, a->handler.tryGet()) { - return &ref.handler == &b; - } - return false; -} - -bool EventTarget::EventHandlerHashCallbacks::matches( - const kj::Own& a, const EventHandler::NativeHandlerRef& b) const { - return matches(a, b.handler); -} - -bool EventTarget::EventHandlerHashCallbacks::matches( - const kj::Own& a, const EventHandler::Handler& b) const { - KJ_SWITCH_ONEOF(b) { - KJ_CASE_ONEOF(jsB, EventHandler::JavaScriptHandler) { - return matches(a, jsB.identity); - } - KJ_CASE_ONEOF(nativeB, EventHandler::NativeHandlerRef) { - return matches(a, nativeB); - } - } - KJ_UNREACHABLE; + return a->identity == b; } uint EventTarget::EventHandlerHashCallbacks::hashCode( @@ -132,32 +49,6 @@ uint EventTarget::EventHandlerHashCallbacks::hashCode( return obj.hashCode(); } -uint EventTarget::EventHandlerHashCallbacks::hashCode(const NativeHandler& handler) const { - return handler.hashCode(); -} - -uint EventTarget::EventHandlerHashCallbacks::hashCode( - const EventHandler::NativeHandlerRef& handler) const { - return hashCode(handler.handler); -} - -uint EventTarget::EventHandlerHashCallbacks::hashCode( - const EventHandler::JavaScriptHandler& handler) const { - return hashCode(handler.identity); -} - -uint EventTarget::EventHandlerHashCallbacks::hashCode(const EventHandler::Handler& handler) const { - KJ_SWITCH_ONEOF(handler) { - KJ_CASE_ONEOF(js, EventHandler::JavaScriptHandler) { - return hashCode(js); - } - KJ_CASE_ONEOF(native, EventHandler::NativeHandlerRef) { - return hashCode(native); - } - } - KJ_UNREACHABLE; -} - jsg::Ref Event::constructor(jsg::Lock& js, kj::String type, jsg::Optional init) { static const Init defaultInit; return js.alloc(kj::mv(type), init.orDefault(defaultInit), Trusted::NO); @@ -198,19 +89,6 @@ jsg::Ref EventTarget::constructor(jsg::Lock& js) { return js.alloc(); } -EventTarget::~EventTarget() noexcept(false) { - for (auto& entry: typeMap) { - for (auto& handler: entry.value.handlers) { - KJ_IF_SOME(native, handler->handler.tryGet()) { - // Note: Can't call `detach()` here because it would loop back and call - // `removeNativeListener()` on us, invalidating the `typeMap` iterator. We'll directly - // null out the state. - native.handler.state = kj::none; - } - } - } -} - size_t EventTarget::getHandlerCount(kj::StringPtr type) const { KJ_IF_SOME(handlerSet, typeMap.find(type)) { return handlerSet.handlers.size(); @@ -259,7 +137,6 @@ void EventTarget::addEventListener(jsg::Lock& js, bool once = false; kj::Maybe> maybeSignal; - kj::Maybe> maybeFollowingSignal; KJ_IF_SOME(value, maybeOptions) { KJ_SWITCH_ONEOF(value) { KJ_CASE_ONEOF(b, bool) { @@ -272,7 +149,6 @@ void EventTarget::addEventListener(jsg::Lock& js, "addEventListener(): options.passive must be false."); once = opts.once.orDefault(false); maybeSignal = kj::mv(opts.signal); - maybeFollowingSignal = kj::mv(opts.followingSignal); } } } @@ -285,44 +161,40 @@ void EventTarget::addEventListener(jsg::Lock& js, } auto maybeAbortHandler = maybeSignal.map([&](jsg::Ref& signal) { - // The returned native handler captures a bare reference to signal and - // will be held by this EventTarget. The signal is the only thing that - // triggers it. If signal is gc'd the native handler created here could - // still be alive which means *technically* it will be holding a bare - // reference for something that is already destroyed. However, there's - // nothing else that would trigger it so it's generally safe-ish. That - // said, it's still a potential UAF so let's guard against it by attaching - // a strong reference to the signal to the event handler. This will mean - // likely keeping the signal in memory longer if it can otherwise be - // gc'd but that's ok, the impact should be minimal. - auto func = - JSG_VISITABLE_LAMBDA((this, type = type.clone(), handler = handler.identity.addRef(js), - signal = signal.addRef()), - (handler, signal), (jsg::Lock& js, jsg::Ref) { - removeEventListener(js, kj::mv(type), kj::mv(handler), kj::none); - }); - - return signal->newNativeHandler(js, kj::str(kAbortEvent), kj::mv(func), true); + // Per the spec's "add an event listener", the {signal} option registers an abort + // algorithm — not an 'abort' listener — that removes this listener when the signal + // aborts. The algorithm lives on the signal, so it captures this EventTarget weakly: + // a strong ref would let a long-lived signal retain every dead target that ever + // registered a listener with it, and a bare `this` would rely on the registration + // handle below never outliving this target. If the target is gone by the time the + // signal aborts, there is nothing left to remove. (The handle is still held by the + // listener's own entry, so in the common case the algorithm is unregistered as soon + // as the listener goes away.) + auto func = JSG_VISITABLE_LAMBDA( + (self = JSG_THIS_WEAK(js), type = type.clone(), handler = handler.identity.addRef(js)), + (handler), (jsg::Lock& js) { + KJ_IF_SOME(target, self.tryGet()) { + target.removeEventListener(js, kj::mv(type), kj::mv(handler), kj::none); + } else { + } + }); + + return signal->addAbortAlgorithm(js, kj::mv(func)); }); - auto eventHandler = kj::heap( - EventHandler::JavaScriptHandler{ - .identity = kj::mv(handler.identity), - .callback = kj::mv(handlerFn), - .abortHandler = kj::mv(maybeAbortHandler), - }, - once); - - // If maybeFollowingSignal is set, we need to attach it to the event handler - // in order to keep it alive. This is used only for AbortSignal.any() where - // the followed signal (this) is being followed by another signal. We need - // to make sure the following signal stays alive until either the followed - // signal is triggered or destroyed. - KJ_IF_SOME(following, maybeFollowingSignal) { - eventHandler = eventHandler.attach(kj::mv(following)); - } + auto eventHandler = kj::heap(EventHandler{ + .identity = kj::mv(handler.identity), + .callback = kj::mv(handlerFn), + .once = once, + .abortHandler = kj::mv(maybeAbortHandler), + }); - getOrCreate(type).handlers.upsert(kj::mv(eventHandler), [&](auto&&...) {}); + auto& handlerSet = getOrCreate(type); + auto sizeBefore = handlerSet.handlers.size(); + handlerSet.handlers.upsert(kj::mv(eventHandler), [&](auto&&...) {}); + if (handlerSet.handlers.size() != sizeBefore) { + listenerCountChanged(js, type, handlerSet.handlers.size()); + } }); } } @@ -346,113 +218,207 @@ void EventTarget::removeEventListener(jsg::Lock& js, KJ_IF_SOME(handler, maybeHandler) { js.withinHandleScope([&] { KJ_IF_SOME(handlerSet, typeMap.find(type)) { - handlerSet.handlers.eraseMatch(handler); + if (handlerSet.handlers.eraseMatch(handler)) { + listenerCountChanged(js, type, handlerSet.handlers.size()); + } } }); } } -void EventTarget::addNativeListener(jsg::Lock& js, NativeHandler& handler) { - auto& set = getOrCreate(handler.type); +EventTarget::EventHandlerSet& EventTarget::getOrCreate(kj::StringPtr type) { + return typeMap.upsert(kj::str(type), EventHandlerSet(), [&](auto&&...) {}).value; +} - auto eventHandler = kj::heap( - EventHandler::NativeHandlerRef{ - .handler = handler, - }, - handler.once); +void EventTarget::addEventHandlerListener(jsg::Lock& js, + kj::StringPtr type, + jsg::HashableV8Ref identity, + HandlerFunction callback) { + auto eventHandler = kj::heap(EventHandler{ + .identity = kj::mv(identity), + .callback = kj::mv(callback), + }); + auto& handlerSet = getOrCreate(type); + auto sizeBefore = handlerSet.handlers.size(); + handlerSet.handlers.upsert(kj::mv(eventHandler), [&](auto&&...) {}); + if (handlerSet.handlers.size() != sizeBefore) { + listenerCountChanged(js, type, handlerSet.handlers.size()); + } +} - set.handlers.upsert(kj::mv(eventHandler), [&](auto&&...) {}); +kj::Maybe EventTarget::getEventHandlerAttribute(jsg::Lock& js, kj::StringPtr type) { + KJ_IF_SOME(attribute, eventHandlerAttributes.find(type)) { + KJ_IF_SOME(handler, attribute.handler) { + return handler.value.getHandle(js); + } + } + return kj::none; } -bool EventTarget::removeNativeListener(EventTarget::NativeHandler& handler) { - KJ_IF_SOME(handlerSet, typeMap.find(handler.type)) { - return handlerSet.handlers.eraseMatch(handler); +EventTarget::EventHandlerAssignment EventTarget::setEventHandlerAttribute(jsg::Lock& js, + kj::StringPtr type, + jsg::Optional> handler) { + const auto getOrCreateAttribute = [&]() -> EventHandlerAttribute& { + return eventHandlerAttributes.findOrCreate( + type, [&] { return decltype(eventHandlerAttributes)::Entry{kj::str(type), {}}; }); + }; + + // Per HTML's event handler semantics: callables (unwrapped as HandlerFunction) become the + // active handler; non-callable objects are retained as the attribute value but are never + // invoked; anything else deactivates the handler (treated as null). + KJ_IF_SOME(h, handler) { + KJ_SWITCH_ONEOF(h) { + KJ_CASE_ONEOF(fn, HandlerFunction) { + auto value = jsg::JsValue( + KJ_ASSERT_NONNULL(fn.tryGetHandle(js.v8Isolate), "handler function has no wrapper")); + auto& attribute = getOrCreateAttribute(); + attribute.handler = EventHandlerAttribute::Handler{ + .value = jsg::JsRef(js, value), + .fn = kj::mv(fn), + }; + activateEventHandlerAttribute(js, type, attribute); + return EventHandlerAssignment::CALLABLE; + } + KJ_CASE_ONEOF(value, jsg::JsValue) { + if (value.isObject()) { + auto& attribute = getOrCreateAttribute(); + attribute.handler = EventHandlerAttribute::Handler{ + .value = jsg::JsRef(js, value), + .fn = kj::none, + }; + activateEventHandlerAttribute(js, type, attribute); + return EventHandlerAssignment::OBJECT; + } + } + } } - return false; + + // Deactivate: clear the value and remove the trampoline listener, so a later reassignment + // takes a fresh position in the listener list. The map entry is kept: its presence is what + // marks the type as managed. + KJ_IF_SOME(attribute, eventHandlerAttributes.find(type)) { + attribute.handler = kj::none; + KJ_IF_SOME(identity, attribute.listenerIdentity) { + removeEventListener(js, kj::str(type), identity.addRef(js), kj::none); + } + attribute.listenerIdentity = kj::none; + } + return EventHandlerAssignment::CLEARED; } -EventTarget::EventHandlerSet& EventTarget::getOrCreate(kj::StringPtr type) { - return typeMap.upsert(kj::str(type), EventHandlerSet(), [&](auto&&...) {}).value; +void EventTarget::activateEventHandlerAttribute( + jsg::Lock& js, kj::StringPtr type, EventHandlerAttribute& attribute) { + // HTML "activate an event handler": if the trampoline listener already exists, the handler + // keeps its current position in the listener list. + if (attribute.listenerIdentity != kj::none) { + return; + } + + auto identity = jsg::HashableV8Ref(js.v8Isolate, v8::Object::New(js.v8Isolate)); + attribute.listenerIdentity = identity.addRef(js); + + // The trampoline is deliberately not the handler itself: it invokes whatever value the + // attribute holds at dispatch time, so reassignment need not (and must not) move it. + auto trampoline = JSG_VISITABLE_LAMBDA((self = JSG_THIS_WEAK(js), type = kj::str(type)), (), + (jsg::Lock & js, jsg::Ref event)->jsg::Optional { + KJ_IF_SOME(target, self.tryGet()) { + KJ_IF_SOME(attribute, target.eventHandlerAttributes.find(type)) { + KJ_IF_SOME(handler, attribute.handler) { + KJ_IF_SOME(fn, handler.fn) { + return fn(js, kj::mv(event)); + } else { + } // Empty elses to squash compiler warnings + } else { + } + } else { + } + } else { + } + return kj::none; + }); + + addEventHandlerListener(js, type, kj::mv(identity), kj::mv(trampoline)); } -bool EventTarget::dispatchEventImpl(jsg::Lock& js, jsg::Ref event) { +namespace { + +// Implements the reporting half of the spec's "inner invoke" step 11 for listener exceptions +// under DispatchExceptionPolicy::REPORT: deliver the exception to the global scope's +// report-an-exception machinery (which fires the cancelable 'error' event, then falls back +// to the console). Outside of a request (e.g. unit-test contexts without a +// ServiceWorkerGlobalScope), fall back to plain console/inspector reporting. +void reportListenerError(jsg::Lock& js, const jsg::JsValue& exception) { + if (IoContext::hasCurrent()) { + IoContext::current().getCurrentLock().getGlobalScope().reportError(js, exception); + } else { + js.reportError(exception); + } +} + +} // namespace + +EventTarget::DispatchResult EventTarget::dispatchEventImpl( + jsg::Lock& js, jsg::Ref event, DispatchExceptionPolicy exceptionPolicy) { event->beginDispatch(JSG_THIS); KJ_DEFER(event->endDispatch()); event->clearPreventDefault(); + kj::Maybe> firstException; + // First, gather all the function handles that we plan to call. This is important to ensure that // the callback can add or remove listeners without affecting the current event's processing. - return js.withinHandleScope([&] { + bool result = js.withinHandleScope([&] { struct Callback { - EventHandler::Handler handler; + // The listener's identity, used to check whether it was removed by an earlier handler + // and to remove it when `once` is set. Old-style on handlers (found via + // property reflection rather than the listener list) have none. + kj::Maybe> identity; + HandlerFunction callback; bool once = false; - bool oldStyle = false; }; kj::Vector callbacks; // Check if there is an `on` property on this object. If so, we treat that as an event - // handler, in addition to the ones registered with addEventListener(). - KJ_IF_SOME(onProp, onEvents.get(js, kj::str("on", event->getType()))) { - // If the on-event is not a function, we silently ignore it rather than raise an error. - KJ_IF_SOME(cb, onProp.tryGet()) { - callbacks.add(Callback{ - .handler = - EventHandler::JavaScriptHandler{ - .identity = nullptr, // won't be used below if oldStyle is true and once is false - .callback = kj::mv(cb), - }, - .oldStyle = true, - }); + // handler, in addition to the ones registered with addEventListener(). This is skipped + // for event types managed as event handler IDL attributes (see + // setEventHandlerAttribute(), e.g. AbortSignal's onabort), whose handlers occupy a + // positioned trampoline listener instead and would otherwise fire twice. + if (!managesEventHandlerAttribute(event->getType())) { + KJ_IF_SOME(onProp, onEvents.get(js, kj::str("on", event->getType()))) { + // If the on-event is not a function, we silently ignore it rather than raise an error. + KJ_IF_SOME(cb, onProp.tryGet()) { + callbacks.add(Callback{ + .identity = kj::none, + .callback = kj::mv(cb), + }); + } } } KJ_IF_SOME(handlerSet, typeMap.find(event->getType())) { callbacks.reserve(handlerSet.handlers.size()); for (auto& handler: handlerSet.handlers.ordered()) { - KJ_SWITCH_ONEOF(handler->handler) { - KJ_CASE_ONEOF(jsh, EventHandler::JavaScriptHandler) { - callbacks.add(Callback{ - .handler = EventHandler::JavaScriptHandler{.identity = jsh.identity.addRef(js), - .callback = jsh.callback.addRef(js)}, - .once = handler->once, - }); - } - KJ_CASE_ONEOF(native, EventHandler::NativeHandlerRef) { - callbacks.add(Callback{ - .handler = - EventHandler::NativeHandlerRef{ - .handler = native.handler, - }, - .once = handler->once, - }); - } - } + callbacks.add(Callback{ + .identity = handler->identity.addRef(js), + .callback = handler->callback.addRef(js), + .once = handler->once, + }); } } - const auto isRemoved = [&](auto& handler) { + const auto isRemoved = [&](jsg::HashableV8Ref& identity) { // This is not the most efficient way to do this but it's what works right now. // Instead of capturing direct references to the handler structs, we copy those // into the Callbacks vector, which means we need to look up the actual handler // again to see if it still exists in the list. The entire way the storage of the // handlers is done here can be improved to make this more efficient. - KJ_IF_SOME(handlerSet, typeMap.find(event->getType())) { - KJ_SWITCH_ONEOF(handler) { - KJ_CASE_ONEOF(js, EventHandler::JavaScriptHandler) { - return handlerSet.handlers.find(js.identity) == kj::none; - } - KJ_CASE_ONEOF(native, EventHandler::NativeHandlerRef) { - return handlerSet.handlers.find(native.handler) == kj::none; - } - } - } else { - return true; + return handlerSet.handlers.find(identity) == kj::none; } - KJ_UNREACHABLE; + return true; }; for (auto& callback: callbacks) { @@ -461,77 +427,103 @@ bool EventTarget::dispatchEventImpl(jsg::Lock& js, jsg::Ref event) { break; } - // If the handler gets removed by an earlier run handler, then we need to - // make sure we don't run it. Skip over and continue. - if (!callback.oldStyle && isRemoved(callback.handler)) { - continue; + KJ_IF_SOME(identity, callback.identity) { + // If the handler was removed by an earlier-run handler, then we need to + // make sure we don't run it. Skip over and continue. + if (isRemoved(identity)) { + continue; + } + + // Per spec ("inner invoke" step 5), once-listeners are removed before invocation. + if (callback.once) { + removeEventListener(js, kj::str(event->getType()), identity.addRef(js), kj::none); + } } - if (callback.once) { - KJ_SWITCH_ONEOF(callback.handler) { - KJ_CASE_ONEOF(jsh, EventHandler::JavaScriptHandler) { - removeEventListener(js, kj::str(event->getType()), jsh.identity.addRef(js), kj::none); + const auto invoke = [&]() { + // Per the standard, the event listener is not supposed to return any value, and + // if it does, that value is ignored. That can be somewhat problematic if the user + // passes an async function as the event handler. Doing so counts as undefined + // behavior and can introduce subtle and difficult to diagnose bugs. Here, if the + // handler does return a value, we're going to emit a warning but otherwise ignore + // it. The warning will only be emitted at most once per EventTarget instance. + auto ret = callback.callback(js, event.addRef()); + KJ_IF_SOME(r, ret) { + auto handle = r.getHandle(js); + // Returning true is the same as calling preventDefault() on the event. + if (handle->IsTrue()) { + event->preventDefault(); } - KJ_CASE_ONEOF(native, EventHandler::NativeHandlerRef) { - // The native handler will handle detaching itself when invoked + if (flags.warnOnHandlerReturn && !handle->IsBoolean()) { + flags.warnOnHandlerReturn = false; + // To help make debugging easier, let's tailor the warning a bit if it was a + // promise. + if (handle->IsPromise()) { + js.logWarning(kj::str( + "An event handler returned a promise that will be ignored. Event handlers " + "should not have a return value and should not be async functions.")); + } else { + js.logWarning(kj::str("An event handler returned a value of type \"", + handle->TypeOf(js.v8Isolate), + "\" that will be ignored. Event handlers should not have a return value.")); + } } } - } + }; - KJ_SWITCH_ONEOF(callback.handler) { - KJ_CASE_ONEOF(jsh, EventHandler::JavaScriptHandler) { - // Per the standard, the event listener is not supposed to return any value, and if it - // does, that value is ignored. That can be somewhat problematic if the user passes an - // async function as the event handler. Doing so counts as undefined behavior and can - // introduce subtle and difficult to diagnose bugs. Here, if the handler does return a - // value, we're going to emit a warning but otherwise ignore it. The warning will only - // be emitted at most once per EventEmitter instance. - auto ret = jsh.callback(js, event.addRef()); - // Note: We used to run each handler in its own v8::TryCatch. However, due to a - // misunderstanding of the V8 API, we incorrectly believed that TryCatch mishandled - // termination (or maybe it actually did at the time), so we changed things such that - // we don't catch exceptions so the first handler to throw an exception terminates the - // loop, and the exception flows out of dispatchEvent(). In theory if multiple - // handlers were registered then maybe we ought to be running all of them even if one - // fails. This isn't entirely clear, though: in the case of 'fetch' handlers, in - // fail-closed mode, an exception from any handler should make the whole request fail, - // but then who cares if the remaining handlers run? Meanwhile, in fail-open mode, for - // consistency, we should probably trigger fallback behavior if any handler throws, so - // again it doesn't matter. For other types of handlers, e.g. WebSocket 'message', it's - // not clear why one would ever register multiple handlers. - KJ_IF_SOME(r, ret) { - auto handle = r.getHandle(js); - // Returning true is the same as calling preventDefault() on the event. - if (handle->IsTrue()) { - event->preventDefault(); - } - if (flags.warnOnHandlerReturn && !handle->IsBoolean()) { - flags.warnOnHandlerReturn = false; - // To help make debugging easier, let's tailor the warning a bit if it was a promise. - if (handle->IsPromise()) { - js.logWarning(kj::str( - "An event handler returned a promise that will be ignored. Event handlers " - "should not have a return value and should not be async functions.")); - } else { - js.logWarning(kj::str("An event handler returned a value of type \"", - handle->TypeOf(js.v8Isolate), - "\" that will be ignored. Event handlers should not have a return value.")); - } + switch (exceptionPolicy) { + case DispatchExceptionPolicy::PROPAGATE: + // The first handler to throw ends the dispatch and the exception flows out of + // dispatchEventImpl(). The runtime's top-level event delivery depends on this: + // for example, a throwing 'fetch' handler must fail the request (fail-closed) + // or trigger fallback (fail-open) rather than let other handlers respond. + invoke(); + break; + case DispatchExceptionPolicy::REPORT: + // Spec "inner invoke" step 11: report the exception and continue with the next + // listener. + JSG_TRY(js) { + invoke(); + } + JSG_CATCH(exception) { + auto handle = jsg::JsValue(exception.getHandle(js)); + if (firstException == kj::none) { + firstException = jsg::JsRef(js, handle); } + reportListenerError(js, handle); } - } - KJ_CASE_ONEOF(native, EventHandler::NativeHandlerRef) { - native.handler(js, event.addRef()); - } + break; } } return !event->isPreventDefault(); }); + + return DispatchResult{.result = result, .firstException = kj::mv(firstException)}; +} + +EventTarget::DispatchExceptionPolicy EventTarget::effectiveExceptionPolicy( + jsg::Lock& js, DispatchExceptionPolicy desired) { + if (desired == DispatchExceptionPolicy::REPORT) { + // Fall back to PROPAGATE when the compat flag is not set or when there is no active + // Worker context (e.g. in C++ unit tests that use the JSG test harness directly). + KJ_IF_SOME(flags, FeatureFlags::tryGet(js)) { + if (!flags.getSpecCompliantDispatchExceptions()) { + return DispatchExceptionPolicy::PROPAGATE; + } + } else { + return DispatchExceptionPolicy::PROPAGATE; + } + } + return desired; } bool EventTarget::dispatchEvent(jsg::Lock& js, jsg::Ref event) { - return dispatchEventImpl(js, kj::mv(event)); + // The JS-exposed dispatchEvent() is a spec surface: listener exceptions are reported and + // do not interrupt the dispatch (nor propagate to the dispatchEvent() caller). + return dispatchEventImpl( + js, kj::mv(event), effectiveExceptionPolicy(js, DispatchExceptionPolicy::REPORT)) + .result; } // A wrapper for the AbortTrigger jsrpc client, that automatically sends a release() message once @@ -571,44 +563,59 @@ class AbortTriggerRpcClient final { rpc::AbortTrigger::Client client; }; +namespace { + +kj::Promise abortRpcClientTask( + IoContext& ioContext, AbortTriggerRpcClient& client, kj::Array reason) { + KJ_IF_SOME(outputLocks, ioContext.waitForOutputLocksIfNecessary()) { + co_await outputLocks; + } + co_await client.abort(reason); +} + +// Sends the serialized abort reason to one RPC clone as a task on the current IoContext, +// which must be the context that owns the client. +void sendAbortToRpc(IoOwn client, kj::Array reason) { + auto& ioContext = IoContext::current(); + // Dereference the IoOwn here, while the owning context is known to be current; the task + // keeps the client alive by holding the IoOwn as an attachment. + auto& clientRef = *client; + ioContext.addTask( + abortRpcClientTask(ioContext, clientRef, kj::mv(reason)).attach(kj::mv(client))); +} + +} // namespace + AbortSignal::AbortSignal(kj::Maybe exception, jsg::Optional> maybeReason, Flag flag) - : canceler(IoContext::current().addObject(kj::refcounted( - exception.map([](const kj::Exception& e) { return e.clone(); })))), - flag(flag), + : flag(flag), + maybeAbortException(kj::mv(exception)), reason(kj::mv(maybeReason)) {} kj::Maybe AbortSignal::getOnAbort(jsg::Lock& js) { - return onAbortHandler.map( - [&](jsg::JsRef& ref) -> jsg::JsValue { return ref.getHandle(js); }); + return getEventHandlerAttribute(js, kAbortEvent); } -void AbortSignal::setOnAbort(jsg::Lock& js, jsg::Optional handler) { - // We only want to accept the handler if it's a valid handler... For anything - // else, set it to null. - KJ_IF_SOME(h, handler) { - if (h.isFunction() || h.isObject()) { - onAbortHandler = jsg::JsRef(js, h); - subscribeToRpcAbort(js); - return; - } - } - onAbortHandler = kj::none; +void AbortSignal::setOnAbort( + jsg::Lock& js, jsg::Optional> handler) { + // The trampoline's activation registers a regular 'abort' listener, which arms the RPC + // abort subscription through listenerCountChanged(). + setEventHandlerAttribute(js, kAbortEvent, kj::mv(handler)); } -void AbortSignal::addEventListener(jsg::Lock& js, - kj::String type, - jsg::Identified handler, - jsg::Optional maybeOptions, - const jsg::TypeHandler>& eventTargetHandler) { - EventTarget::addEventListener( - js, kj::mv(type), kj::mv(handler), kj::mv(maybeOptions), eventTargetHandler); - subscribeToRpcAbort(js); +void AbortSignal::listenerCountChanged(jsg::Lock& js, kj::StringPtr type, size_t count) { + // Only 'abort' listeners can observe an abort; registrations for other event types must + // not arm the RPC subscription (whose pending awaitIo blocks actor hibernation). A + // notification that leaves 'abort' listeners registered arms too — arming is idempotent, + // and any such notification means an abort could still be observed. + if (type == kAbortEvent && count > 0) { + subscribeToRpcAbort(js); + } } bool AbortSignal::getAborted(jsg::Lock& js) { - return canceler->isCanceled() || hasPendingReason(); + return maybeAbortException != kj::none || hasPendingReason(); } jsg::JsValue AbortSignal::getReason(jsg::Lock& js) { @@ -648,7 +655,7 @@ jsg::Ref AbortSignal::abort(jsg::Lock& js, jsg::OptionalisCanceled()) { + if (maybeAbortException != kj::none) { KJ_IF_SOME(r, reason) { js.throwException(r.getHandle(js)); } else { @@ -682,67 +689,232 @@ jsg::Ref AbortSignal::timeout(jsg::Lock& js, double delay) { return kj::mv(signal); } -jsg::Ref AbortSignal::any(jsg::Lock& js, - kj::Array> signals, - const jsg::TypeHandler& handler, - const jsg::TypeHandler>& eventTargetHandler) { +jsg::Ref AbortSignal::any(jsg::Lock& js, kj::Array> signals) { + // Implements the spec's "create a dependent abort signal". + // If nothing was passed in, we can just return a signal that never aborts. if (signals.size() == 0) { return js.alloc(kj::none, kj::none, AbortSignal::Flag::NEVER_ABORTS); } - // Let's check to see if any of the signals are already aborted. If it is, we can - // optimize here by skipping the event handler registration. + // Spec step 2: if any of the signals is already aborted, return an already-aborted signal + // carrying its reason; nothing gets linked. for (auto& sig: signals) { if (sig->getAborted(js)) { return AbortSignal::abort(js, sig->getReason(js)); } } - // Otherwise we need to create a new signal and register event handlers on all - // of the signals that were passed in. - auto signal = js.alloc(); + auto resultSignal = js.alloc(); + resultSignal->dependent = true; + + // Links resultSignal as a dependent of the given (never itself dependent) source. + // Duplicate links are harmless: a dependent is only aborted once, so extra entries are + // skipped at trigger time — matching the spec's set semantics observably. + const auto linkToSource = [&](AbortSignal& source) { + source.dependentSignals.add(resultSignal.addRef()); + resultSignal->sourceSignals.add(source.getWeakRefToThis(js)); + + // A dependent must observe aborts that arrive for the source over RPC, just like any + // other abort observer. + source.subscribeToRpcAbort(js); + }; + + // Spec step 4, including the flattening rule: a source that is itself dependent + // contributes its own sources rather than itself, so dependency chains never form. (A + // dead flattened source can never abort and so contributes nothing.) for (auto& sig: signals) { - // This is a bit of a hack. We want to call addEventListener, but that requires a - // jsg::Identified, which we can't create directly yet. - // So we create a jsg::Function, wrap that in a v8::Function, then convert that into - // the jsg::Identified, and voila, we have what we need. - auto fn = js.wrapSimpleFunction(js.v8Context(), - [signal = signal.addRef(), self = sig.getWeakRef(js)](jsg::Lock& js, auto&) mutable { - // Keep the returned signal alive while this listener is running. EventTarget removes - // once-handlers before invocation, which otherwise drops `followingSignal` before nested - // JS/V8 work in triggerAbort() can run GC. - signal->triggerAbort(js, self->getReason(js)); - }); - jsg::Identified identified = {.identity = {js.v8Isolate, fn}, - .unwrapped = JSG_REQUIRE_NONNULL(handler.tryUnwrap(js, fn.As()), TypeError, - "Unable to create AbortSignal.any handler")}; - - sig->addEventListener(js, kj::str(kAbortEvent), kj::mv(identified), - AddEventListenerOptions{// Once the abort is triggered, this handler should remove itself. - .once = true, - // Each of the followed signals will maintain a strong reference to this new - // one that's been created. - .followingSignal = signal.addRef()}, - eventTargetHandler); + if (!sig->dependent) { + linkToSource(*sig); + } else { + for (auto& weakSource: sig->sourceSignals) { + KJ_IF_SOME(source, weakSource.tryGet()) { + linkToSource(source); + } + } + } } - return signal; + + return resultSignal; } void AbortSignal::visitForGc(jsg::GcVisitor& visitor) { - visitor.visit(reason, onAbortHandler); + visitor.visit(reason); + for (auto& algorithm: abortAlgorithms) { + visitor.visit(algorithm.fn); + } + for (auto& dep: dependentSignals) { + visitor.visit(dep); + } } -RefcountedCanceler& AbortSignal::getCanceler() { - return *canceler; +namespace { + +// Takes a cell's slot content, leaving the slot empty. Callers must use the taken value +// only after this returns, i.e. after the cell's mutex has been released. +template +kj::Maybe take(const kj::MutexGuarded>& slot) { + auto lock = slot.lockExclusive(); + auto value = kj::mv(*lock); + *lock = kj::none; + return value; +} + +// Reclaims registration cells whose consumer is gone (slot already cleared) or whose owning +// IoContext has been destroyed (the slot content could never be used anyway; clearing it +// here is safe from any thread because dropping an IoOwn on a defunct context is a no-op and +// abort actions capture nothing needing the owner). Called on registration paths so that +// growth on a long-lived signal is bounded by its live registrations. `slotOf` maps a cell +// to its guarded slot. +template +void sweepCells(kj::Vector>& cells, SlotOf slotOf) { + size_t dst = 0; + for (size_t i = 0; i < cells.size(); i++) { + bool dead = [&]() { + auto lock = slotOf(*cells[i]).lockExclusive(); + if (*lock == kj::none) { + return true; + } + if (cells[i]->executor.isTargetDestroyed()) { + *lock = kj::none; + return true; + } + return false; + }(); + if (!dead) { + if (dst != i) { + cells[dst] = kj::mv(cells[i]); + } + ++dst; + } + } + cells.truncate(dst); } -void AbortSignal::triggerAbort( - jsg::Lock& js, jsg::Optional> maybeReason) { - KJ_ASSERT(flag != Flag::NEVER_ABORTS); - if (canceler->isCanceled()) { - return; +} // namespace + +kj::Own AbortSignal::addAbortAction( + jsg::Lock& js, kj::Function action) { + // The RAII registration handle. Clearing the cell's slot is the only thing it does, which + // makes it safe to drop from any thread; the emptied cell itself is removed from the + // signal by a later sweep under the isolate lock. + class Registration final { + public: + Registration(kj::Arc cell): cell(kj::mv(cell)) {} + ~Registration() noexcept(false) { + *cell->action.lockExclusive() = kj::none; + } + KJ_DISALLOW_COPY_AND_MOVE(Registration); + + private: + kj::Arc cell; + }; + + if (getNeverAborts()) { + return kj::Own(); } + + // Abort actions observe aborts, including ones arriving over RPC for a deserialized + // signal; this is the single arming point for every native registration path (wrap(), + // newCanceler(), and direct callers alike). + subscribeToRpcAbort(js); + + auto& ioContext = IoContext::current(); + + sweepCells(nativeRegistrations, [](auto& cell) -> auto& { return cell.action; }); + + auto cell = kj::arc(ioContext.getCrossContextExecutor(), kj::mv(action)); + nativeRegistrations.add(cell.addRef()); + return kj::heap(kj::mv(cell)); +} + +kj::Own AbortSignal::addAbortAlgorithm(jsg::Lock& js, jsg::Function algorithm) { + // The RAII registration handle. It holds only a weak reference: if the signal is already + // gone (or aborted, which empties the algorithm list), unregistration is a no-op. + class Registration final { + public: + Registration(jsg::WeakRef signal, uint64_t token) + : signal(kj::mv(signal)), + token(token) {} + ~Registration() noexcept(false) { + KJ_IF_SOME(s, signal.tryGet()) { + s.removeAbortAlgorithm(token); + } + } + KJ_DISALLOW_COPY_AND_MOVE(Registration); + + private: + jsg::WeakRef signal; + uint64_t token; + }; + + if (getNeverAborts()) { + return kj::Own(); + } + + subscribeToRpcAbort(js); + + auto token = nextAbortAlgorithmToken++; + abortAlgorithms.add(AbortAlgorithm{.token = token, .fn = kj::mv(algorithm)}); + return kj::heap(JSG_THIS_WEAK(js), token); +} + +void AbortSignal::removeAbortAlgorithm(uint64_t token) { + // Linear scan-and-shift: the list is short and the remaining entries' order must be + // preserved (algorithms run in registration order). + for (size_t i = 0; i < abortAlgorithms.size(); i++) { + if (abortAlgorithms[i].token == token) { + for (size_t j = i; j + 1 < abortAlgorithms.size(); j++) { + abortAlgorithms[j] = kj::mv(abortAlgorithms[j + 1]); + } + abortAlgorithms.removeLast(); + return; + } + } +} + +kj::Own AbortSignal::registerPendingCancellation(jsg::Lock& js, ReleasingCanceler& canceler) { + // Capturing by reference is safe: the returned handle guarantees the action never runs + // once the handle has been destroyed, and holders destroy the handle before the canceler. + return addAbortAction(js, + [&canceler](jsg::Lock& js, const kj::Exception& exception) { canceler.cancel(exception); }); +} + +AbortSignal::Cancellation AbortSignal::newCanceler(jsg::Lock& js) { + if (getNeverAborts()) { + return { + .canceler = kj::heap(), + .registration = kj::Own(), + }; + } + + if (getAborted(js)) { + // Already aborted: hand back a pre-canceled canceler; there is no future abort to hook. + auto exception = [&]() -> kj::Exception { + KJ_IF_SOME(e, maybeAbortException) { + return e.clone(); + } + KJ_IF_SOME(r, deserializePendingReason(js)) { + return js.exceptionToKj(r); + } + KJ_UNREACHABLE; + }(); + return { + .canceler = kj::heap(kj::mv(exception)), + .registration = kj::Own(), + }; + } + + auto canceler = kj::heap(); + auto registration = registerPendingCancellation(js, *canceler); + return { + .canceler = kj::mv(canceler), + .registration = kj::mv(registration), + }; +} + +void AbortSignal::setAbortState( + jsg::Lock& js, jsg::Optional> maybeReason) { auto exception = AbortSignal::abortException(js, maybeReason); KJ_IF_SOME(r, maybeReason) { KJ_SWITCH_ONEOF(r) { @@ -756,22 +928,146 @@ void AbortSignal::triggerAbort( } else { reason = js.exceptionToJsValue(exception.clone()); } + maybeAbortException = kj::mv(exception); +} + +void AbortSignal::severSources(jsg::Lock& js) { + auto sources = kj::mv(sourceSignals); + for (auto& weakSource: sources) { + KJ_IF_SOME(source, weakSource.tryGet()) { + auto& deps = source.dependentSignals; + for (size_t i = 0; i < deps.size(); i++) { + if (deps[i].get() == this) { + for (size_t j = i; j + 1 < deps.size(); j++) { + deps[j] = kj::mv(deps[j + 1]); + } + deps.removeLast(); + break; + } + } + } + } +} + +void AbortSignal::triggerAbort( + jsg::Lock& js, jsg::Optional> maybeReason) { + KJ_ASSERT(flag != Flag::NEVER_ABORTS); + if (maybeAbortException != kj::none) { + return; + } + + // Spec "signal abort" steps 1-2: record the abort reason. + setAbortState(js, kj::mv(maybeReason)); + + // Spec steps 3-4: record the reason on every not-yet-aborted dependent signal NOW — before + // any abort steps or events run anywhere — and collect them; their own abort steps run + // only after ours complete (step 6). Recording the state as part of collection means a + // signal linked more than once (e.g. any([s, s])) fails the not-yet-aborted check on the + // second encounter and is collected — and has its abort steps run — only once. Collect + // with fresh strong addRef()s rather than by moving the stored refs: the stored refs are + // GC-traced, and a ref moved out of its visited home would leave the dependent's wrapper + // collectable by any GC that runs during the JS work below (reason derivation and event + // dispatch can both run arbitrary JS). Clearing the member also severs the links: an + // aborted signal has no further use for its dependents, and any() never links to an + // aborted source, so nothing new can arrive. + kj::Vector> dependentsToAbort; + if (!dependentSignals.empty()) { + auto reasonHandle = KJ_ASSERT_NONNULL(reason).getHandle(js); + for (auto& dep: dependentSignals) { + if (dep->maybeAbortException == kj::none) { + dep->setAbortState(js, kj::OneOf(reasonHandle)); + dependentsToAbort.add(dep.addRef()); + } + } + dependentSignals.clear(); + } + + // Spec step 5: run our own abort steps. + runAbortSteps(js); + + // Spec step 6: run each collected dependent's abort steps, unlinking each from any other + // sources it still has (those can no longer abort it, nor need they keep it alive). + for (auto& dep: dependentsToAbort) { + dep->severSources(js); + dep->runAbortSteps(js); + } +} + +void AbortSignal::runAbortSteps(jsg::Lock& js) { + auto& exception = KJ_ASSERT_NONNULL(maybeAbortException); + + // 1. Abort algorithms (spec "run the abort steps", steps 1-2): run each algorithm in + // registration order, then empty the list. The functions are copied out with fresh + // strong addRef()s (same pattern as dispatchEventImpl): the stored ones are GC-traced, + // and an algorithm may itself run JS — and therefore GC — while later entries await + // their turn. Emptying the list first also makes the loop safe against re-entrant + // mutation (e.g. an algorithm's effects dropping another algorithm's registration). + if (!abortAlgorithms.empty()) { + kj::Vector> algorithms; + algorithms.reserve(abortAlgorithms.size()); + for (auto& algorithm: abortAlgorithms) { + algorithms.add(algorithm.fn.addRef(js)); + } + abortAlgorithms.clear(); + for (auto& algorithm: algorithms) { + algorithm(js); + } + } + + // 2. Native cancellations (canceler-wrapped promises and abort actions). Each registration + // runs in the IoContext that created it: synchronously if that context is the current + // one, otherwise delivered on that context's next turn — or dropped, if that context is + // already gone (in which case everything it wanted to cancel died with it). Taking the + // vector up front makes this safe against re-entrant registration. + // + // A deferred delivery does not take the action with it: it re-takes the cell's slot on + // arrival in the owning context, so that a consumer (and the references its action + // captures) that went away in the meantime reliably turns the delivery into a no-op. + auto cells = kj::mv(nativeRegistrations); + for (auto& cell: cells) { + if (cell->executor.isCurrent()) { + KJ_IF_SOME(action, take(cell->action)) { + action(js, exception); + } + } else { + cell->executor.tryExecute( + [cell = cell.addRef(), ex = exception.clone()](jsg::Lock& js) mutable { + KJ_IF_SOME(action, take(cell->action)) { + action(js, ex); + } + }); + } + } - canceler->cancel(kj::mv(exception)); + // 3. Dispatch to RPC clients, with the same per-registration routing and re-take. + if (!rpcRegistrations.empty()) { + auto regs = kj::mv(rpcRegistrations); - // 1. Dispatch to RPC clients - if (!rpcClients.empty()) { - IoContext& ioContext = IoContext::current(); + // Serialize the reason once; each clone gets its own copy of the bytes. jsg::Serializer ser(js); KJ_IF_SOME(r, reason) { ser.write(js, r.getHandle(js)); } - auto released = ser.release(); - ioContext.addTask(sendToRpc(kj::mv(released.data))); + + for (auto& reg: regs) { + auto bytes = kj::heapArray(released.data); + if (reg->executor.isCurrent()) { + KJ_IF_SOME(client, take(reg->client)) { + sendAbortToRpc(kj::mv(client), kj::mv(bytes)); + } + } else { + reg->executor.tryExecute( + [reg = reg.addRef(), bytes = kj::mv(bytes)](jsg::Lock& js) mutable { + KJ_IF_SOME(client, take(reg->client)) { + sendAbortToRpc(kj::mv(client), kj::mv(bytes)); + } + }); + } + } } - // 2. Dispatch to local listeners + // 4. Dispatch to local listeners // This is questionable only because it goes against the spec but it does help prevent // memory leaks. Once the abort signal has been triggered, there's really nothing else @@ -781,7 +1077,10 @@ void AbortSignal::triggerAbort( // of the spec here should be just fine. KJ_DEFER(removeAllHandlers()); - dispatchEventImpl(js, js.alloc(kAbortEvent)); + // Per spec, "signal abort" cannot throw: listener exceptions are reported, and the + // remaining listeners (and, for a source signal, the dependents' abort steps) still run. + dispatchEventImpl(js, js.alloc(kAbortEvent, Event::Init{}, Trusted::YES), + effectiveExceptionPolicy(js, DispatchExceptionPolicy::REPORT)); } void AbortSignal::serialize(jsg::Lock& js, jsg::Serializer& serializer) { @@ -795,13 +1094,12 @@ void AbortSignal::serialize(jsg::Lock& js, jsg::Serializer& serializer) { JSG_REQUIRE( externalHandler != nullptr, DOMDataCloneError, "AbortSignal can only be serialized for RPC."); - serializer.writeRawUint32(static_cast(canceler->isCanceled())); + serializer.writeRawUint32(static_cast(getAborted(js))); serializer.writeRawUint32(static_cast(flag)); - KJ_IF_SOME(r, reason) { - serializer.write(js, r.getHandle(js)); - } else { - serializer.write(js, js.undefined()); - } + // getReason() falls back to a pending (RPC-received but not yet triggered) reason, so a + // deserialized signal re-serialized before its receiving request processed the abort still + // carries the reason along; it returns undefined when there is none. + serializer.write(js, getReason(js)); if (getAborted(js) || getNeverAborts()) { // This AbortSignal cannot be triggered in the future. No stream is needed. @@ -822,9 +1120,13 @@ void AbortSignal::serialize(jsg::Lock& js, jsg::Serializer& serializer) { }(); auto& ioContext = IoContext::current(); + + sweepCells(rpcRegistrations, [](auto& cell) -> auto& { return cell.client; }); + // Keep track of every AbortSignal cloned from this one. - // If this->triggerAbort(...) is called, each rpcClient will be informed. - rpcClients.add(ioContext.addObject(kj::heap(kj::mv(triggerCap)))); + // If this->triggerAbort(...) is called, each clone will be informed. + rpcRegistrations.add(kj::arc(ioContext.getCrossContextExecutor(), + ioContext.addObject(kj::heap(kj::mv(triggerCap))))); } jsg::Ref AbortSignal::deserialize( @@ -859,38 +1161,37 @@ jsg::Ref AbortSignal::deserialize( auto resolvedSignal = ioctx.getExternalPusher()->unwrapAbortSignal(reader.getAbortSignal()); + signal->rpcReceiverContext = ioctx.getCrossContextExecutor(); signal->rpcAbortPromise = ioctx.addObject(kj::heap(kj::mv(resolvedSignal.signal))); - signal->pendingReason = ioctx.addObject(kj::mv(resolvedSignal.reason)); + signal->pendingReason = kj::mv(resolvedSignal.reason); return signal; } +int AbortSignal::getNativeRegistrationCountForTest() { + return static_cast(nativeRegistrations.size() + rpcRegistrations.size()); +} + void AbortSignal::skipReleaseForTest() { - for (auto& cap: rpcClients) { - cap->skipReleaseForTest = true; + for (auto& reg: rpcRegistrations) { + KJ_IF_SOME(client, take(reg->client)) { + client->skipReleaseForTest = true; + } } - rpcClients.clear(); + rpcRegistrations.clear(); } -kj::Promise AbortSignal::sendToRpc(kj::Array&& reason) { - auto& ioContext = IoContext::current(); - - KJ_IF_SOME(outputLocks, ioContext.waitForOutputLocksIfNecessary()) { - co_await outputLocks; +bool AbortSignal::isRpcReceiverContextCurrent() { + KJ_IF_SOME(executor, rpcReceiverContext) { + return executor.isCurrent(); } - - kj::Vector> promises; - for (auto& cap: rpcClients) { - promises.add(cap->abort(reason)); - } - - co_await kj::joinPromises(promises.releaseAsArray()); + return false; } bool AbortSignal::hasPendingReason() { KJ_IF_SOME(pr, pendingReason) { - return *pr != nullptr; + return *pr->value.lockShared() != nullptr; } return false; @@ -898,20 +1199,37 @@ bool AbortSignal::hasPendingReason() { kj::Maybe AbortSignal::deserializePendingReason(jsg::Lock& js) { KJ_IF_SOME(pr, pendingReason) { - if (*pr == nullptr) { - // pendingReason not initialized. This means abort wasn't yet triggered - return kj::none; - } - - KJ_SWITCH_ONEOF(*pr) { - KJ_CASE_ONEOF(v8Serialized, kj::Array) { - jsg::Deserializer des(js, v8Serialized); - return kj::some(des.readValue(js)); + // Copy the pending state out under the mutex; the JS work below then runs without + // holding it. (The box is written at most once, by the receiving request; it may be read + // from any context.) + auto pending = [&]() -> kj::Maybe { + auto lock = pr->value.lockShared(); + if (*lock == nullptr) { + // pendingReason not initialized. This means abort wasn't yet triggered. + return kj::none; } + KJ_SWITCH_ONEOF(*lock) { + KJ_CASE_ONEOF(v8Serialized, kj::Array) { + return ExternalPusherImpl::PendingAbortReason(kj::heapArray(v8Serialized)); + } + KJ_CASE_ONEOF(exception, kj::Exception) { + return ExternalPusherImpl::PendingAbortReason(exception.clone()); + } + } + KJ_UNREACHABLE; + }(); - KJ_CASE_ONEOF(exception, kj::Exception) { - return kj::some(js.exceptionToJsValue(exception.clone()).getHandle(js)); + KJ_IF_SOME(p, pending) { + KJ_SWITCH_ONEOF(p) { + KJ_CASE_ONEOF(v8Serialized, kj::Array) { + jsg::Deserializer des(js, v8Serialized); + return kj::some(des.readValue(js)); + } + KJ_CASE_ONEOF(exception, kj::Exception) { + return kj::some(js.exceptionToJsValue(kj::mv(exception)).getHandle(js)); + } } + KJ_UNREACHABLE; } } @@ -923,6 +1241,29 @@ void AbortSignal::subscribeToRpcAbort(jsg::Lock& js) { // we want to arrange to awaitIo() for the underlying RPC signal. If no one is actually listening, // though, we don't want to awaitIo() since it blocks hibernation in actors. + if (rpcAbortPromise == kj::none) { + // Not an RPC-received signal, or the subscription is already armed. + return; + } + + if (!isRpcReceiverContextCurrent()) { + // Only the request that deserialized this signal can arm the subscription — it owns the + // underlying promise — but a signal retained across requests may see registrations from + // other contexts. Ask the receiving context to arm on its next turn. The queued action + // captures only a WeakRef, which is safe to destroy from any thread should that context + // be torn down without draining its queue. If the context is already gone, the request + // is dropped: no delivery is possible anymore, though the abort itself stays observable + // through the pending-reason box (see docs/reference/detail/abort-signal.md). + KJ_IF_SOME(executor, rpcReceiverContext) { + executor.tryExecute([weakSelf = JSG_THIS_WEAK(js)](jsg::Lock& js) { + KJ_IF_SOME(self, weakSelf.tryGet()) { + self.subscribeToRpcAbort(js); + } + }); + } + return; + } + KJ_IF_SOME(promise, rpcAbortPromise) { IoContext::current().awaitIo(js, kj::mv(*promise), [self = JSG_THIS](jsg::Lock& js) mutable { KJ_IF_SOME(r, self->deserializePendingReason(js)) { @@ -949,36 +1290,21 @@ void AbortController::abort(jsg::Lock& js, jsg::Optional maybeReas } void EventTarget::visitForGc(jsg::GcVisitor& visitor) { - visitor.visit(maybeListenerCallback); for (auto& entry: typeMap) { for (auto& handler: entry.value.handlers) { - KJ_SWITCH_ONEOF(handler->handler) { - KJ_CASE_ONEOF(js, EventHandler::JavaScriptHandler) { - visitor.visit(js); - } - KJ_CASE_ONEOF(native, EventHandler::NativeHandlerRef) { - // Note that even though `native.handler` is a non-owned reference, we still need to - // visit it. This is because we are the ones that will invoke the handles contained - // in the native handler if it ever fires. The actual owner of the C++ NativeHandler - // object doesn't ever access the JS objects it contains; the ownership relationship - // exists only for RAII reasons, so that the NativeHandler is automatically unregistered - // if the owner is destroyed. - // - // You might say: "Well, it's fine if the owner is responsible for visiting it, because - // if the owner is no longer reachable then it will be destroyed and it will unregister - // itself from here!" That doesn't quite work: V8's GC doesn't necessarily destroy - // objects immediately when they become unreachable. However, it is no longer safe to - // access an object once it is unreachable. Therefore, if we left it to the - // NativeHandler's owner to visit the object, it's possible that the object becomes - // poison some time before it is actually unregistered. - // - // Put another way, this is a very weird case where the C++ ownership and the JavaScript - // ownership are different. We need GC visitation to follow the JavaScript ownership - // graph. - visitor.visit(native.handler); - } + visitor.visit(*handler); + } + } + for (auto& entry: eventHandlerAttributes) { + KJ_IF_SOME(handler, entry.value.handler) { + visitor.visit(handler.value); + KJ_IF_SOME(fn, handler.fn) { + visitor.visit(fn); } } + KJ_IF_SOME(identity, entry.value.listenerIdentity) { + visitor.visit(identity); + } } } @@ -1058,32 +1384,15 @@ CustomEvent::CustomEventInit::operator Event::Init() { }; } -size_t EventTarget::EventHandler::JavaScriptHandler::jsgGetMemorySelfSize() const { - return sizeof(JavaScriptHandler); -} - -void EventTarget::EventHandler::JavaScriptHandler::jsgGetMemoryInfo( - jsg::MemoryTracker& tracker) const { - tracker.trackField("identity", identity); - tracker.trackField("callback", callback); - if (abortHandler != kj::none) { - tracker.trackFieldWithSize( - "abortHandler", sizeof(kj::Own) + sizeof(NativeHandler)); - } -} - size_t EventTarget::EventHandler::jsgGetMemorySelfSize() const { return sizeof(EventHandler); } void EventTarget::EventHandler::jsgGetMemoryInfo(jsg::MemoryTracker& tracker) const { - KJ_SWITCH_ONEOF(handler) { - KJ_CASE_ONEOF(js, JavaScriptHandler) { - tracker.trackField("js", js); - } - KJ_CASE_ONEOF(native, NativeHandlerRef) { - tracker.trackFieldWithSize("native", sizeof(NativeHandlerRef)); - } + tracker.trackField("identity", identity); + tracker.trackField("callback", callback); + if (abortHandler != kj::none) { + tracker.trackFieldWithSize("abortHandler", sizeof(kj::Own)); } } @@ -1099,6 +1408,8 @@ void EventTarget::EventHandlerSet::jsgGetMemoryInfo(jsg::MemoryTracker& tracker) void EventTarget::visitForMemoryInfo(jsg::MemoryTracker& tracker) const { tracker.trackField("typeMap", typeMap); + tracker.trackFieldWithSize("eventHandlerAttributes", + eventHandlerAttributes.size() * sizeof(decltype(eventHandlerAttributes)::Entry)); } } // namespace workerd::api diff --git a/src/workerd/api/basics.h b/src/workerd/api/basics.h index 474443bbf95..dd8daa8a249 100644 --- a/src/workerd/api/basics.h +++ b/src/workerd/api/basics.h @@ -37,7 +37,8 @@ class Event: public jsg::Object { JSG_STRUCT(bubbles, cancelable, composed); }; - inline explicit Event(kj::String ownType, Init init = {}, Trusted trusted = Trusted::YES) + // Only events constructed by the runtime itself may pass Trusted::YES. + inline explicit Event(kj::String ownType, Init init = {}, Trusted trusted = Trusted::NO) : ownType(kj::mv(ownType)), type(this->ownType) { flags.trusted = trusted == Trusted::YES; @@ -46,7 +47,7 @@ class Event: public jsg::Object { flags.composed = init.composed.orDefault(false); } - inline explicit Event(kj::StringPtr type, Init init = {}, Trusted trusted = Trusted::YES) + inline explicit Event(kj::StringPtr type, Init init = {}, Trusted trusted = Trusted::NO) : type(type) { flags.trusted = trusted == Trusted::YES; flags.bubbles = init.bubbles.orDefault(false); @@ -130,9 +131,9 @@ class Event: public jsg::Object { return 0.0; } - // What makes an Event trusted? It's pretty simple... any Event created - // by EW internally is Trusted, any Event created using new Event() in JS - // is not trusted. + // Per the spec, an event is trusted iff it was constructed and dispatched by the runtime + // itself. The Event constructor defaults to untrusted; runtime construction sites (and + // runtime-only subclass constructors) opt in by passing Trusted::YES explicitly. inline bool getIsTrusted() const { return flags.trusted; } @@ -228,7 +229,7 @@ class Event: public jsg::Object { kj::Maybe> target; struct Flags { - uint8_t trusted : 1 = 1; + uint8_t trusted : 1 = 0; uint8_t stopped : 1 = 0; uint8_t preventedDefault : 1 = 0; uint8_t isBeingDispatched : 1 = 0; @@ -246,7 +247,8 @@ class Event: public jsg::Object { class ExtendableEvent: public Event { public: - using Event::Event; + // Runtime-only (the JS constructor is deleted); always trusted. + explicit ExtendableEvent(kj::StringPtr type): Event(type, {}, Trusted::YES) {} // While ExtendableEvent is defined by the spec to be constructable, there's really not a // lot of reason currently to do so, especially with the restriction that waitUntil can @@ -313,27 +315,63 @@ class CustomEvent: public Event { // An implementation of the Web Platform Standard EventTarget API class EventTarget: public jsg::Object { public: - ~EventTarget() noexcept(false); - size_t getHandlerCount(kj::StringPtr type) const; kj::Array getHandlerNames() const; - bool dispatchEventImpl(jsg::Lock& js, jsg::Ref event); + // What to do when a listener throws during dispatch. + // + // PROPAGATE is the historical workerd behavior: the first throwing listener ends the + // dispatch and the exception flows out of dispatchEventImpl(). The runtime's own top-level + // event delivery (fetch/scheduled/etc.) relies on this for its failure semantics, so it + // remains the default for internal callers. + // + // REPORT is the behavior the spec requires of the JS-observable surfaces ("inner invoke" + // step 11: report the exception and continue with the next listener): the exception is + // delivered to the global scope's report-an-exception machinery (the global 'error' + // event, then console logging) and the dispatch continues. Used by the JS-exposed + // dispatchEvent(), by AbortSignal aborts (which the spec forbids from throwing), by the + // global scope's own reportError(), and by the UA-fired dispatches of WebSocket, + // EventSource, and MessagePort — the latter additionally apply a fail-fast reaction via + // DispatchResult::firstException. + enum class DispatchExceptionPolicy { PROPAGATE, REPORT }; + + // The result of a dispatchEventImpl() call. + struct DispatchResult { + // Per the spec's dispatch algorithm: false iff the event is cancelable and a listener + // called preventDefault(). + bool result; + + // Under DispatchExceptionPolicy::REPORT, the first listener exception, if any listener + // threw. It has already been reported; it is surfaced here for native callers that + // additionally apply a fail-fast reaction on top of the spec's report-and-continue + // dispatch (e.g. erroring out a WebSocket whose listener threw). + kj::Maybe> firstException; + }; + + // Returns REPORT if the caller requested REPORT and the spec_compliant_dispatch_exceptions + // compat flag is enabled; falls back to PROPAGATE otherwise. PROPAGATE requests are + // returned as-is. + static DispatchExceptionPolicy effectiveExceptionPolicy( + jsg::Lock& js, DispatchExceptionPolicy desired); + + DispatchResult dispatchEventImpl(jsg::Lock& js, + jsg::Ref event, + DispatchExceptionPolicy exceptionPolicy = DispatchExceptionPolicy::PROPAGATE); inline void removeAllHandlers() { typeMap.clear(); + // Any activated event handler attribute trampolines were just dropped along with the + // listener list; clear their identities so a later assignment activates afresh. + for (auto& entry: eventHandlerAttributes) { + entry.value.listenerIdentity = kj::none; + } } inline void enableWarningOnSpecialEvents() { flags.warnOnSpecialEvents = true; } - // The EventListenerCallback, if given, is called whenever addEventListener - // or removeEventListener is invoked to report the number of registered - // handlers for the event. - using EventListenerCallback = jsg::Function; - // --------------------------------------------------------------------------- // JS API @@ -350,12 +388,6 @@ class EventTarget: public jsg::Object { jsg::Optional> signal; JSG_STRUCT(capture, passive, once, signal); - - // A following signal is used when the EventTarget is an AbortSignal - // that is being followed by another AbortSignal via the AbortSignal.any. - // This is used to keep the following signal alive until either the - // signal is triggered or this AbortSignal is destroyed. - jsg::Optional> followingSignal; }; using AddEventListenerOpts = kj::OneOf; @@ -408,106 +440,63 @@ class EventTarget: public jsg::Object { static jsg::Ref constructor(jsg::Lock& js); - // Registers a lambda that will be called when the given event type is emitted. - // The handler will be registered for as long as the returned kj::Own - // handle is held. If the EventTarget is destroyed while the native handler handle - // is held, it will be automatically detached. - // - // The caller must not do anything with the returned Own except drop it. This is why it - // is Own and not Own. - kj::Own newNativeHandler( - jsg::Lock& js, kj::String type, jsg::Function)> func, bool once = false); - void visitForMemoryInfo(jsg::MemoryTracker& tracker) const; protected: - void setEventListenerCallback(EventListenerCallback&& callback) { - maybeListenerCallback = kj::mv(callback); - } + // Invoked whenever the number of registered listeners for `type` changes: on + // addEventListener() and removeEventListener() — including once-listener removal during + // dispatch and {signal}-triggered removals — and on managed event handler attribute + // activation and deactivation, whose trampoline occupies a regular listener slot. + // removeAllHandlers() does not notify: it is only used for wholesale teardown. + virtual void listenerCountChanged(jsg::Lock& js, kj::StringPtr type, size_t count) {} + + // The result of a setEventHandlerAttribute() assignment: cleared, or activated with a + // non-callable object, or activated with a callable handler. + enum class EventHandlerAssignment { CLEARED, OBJECT, CALLABLE }; + + // Implement HTML's event handler IDL attribute semantics for an on attribute (e.g. + // AbortSignal's onabort): assigning any object activates a trampoline listener that + // occupies a normal position in the listener list (kept across reassignment; a fresh + // position after deactivation), and assigning anything else deactivates it. Only callable + // values are ever invoked: the trampoline invokes whatever callable the attribute holds at + // dispatch time. Dispatch does not additionally consult the legacy on property + // reflection for a managed type. The getter returns the exact value assigned. + kj::Maybe getEventHandlerAttribute(jsg::Lock& js, kj::StringPtr type); + EventHandlerAssignment setEventHandlerAttribute(jsg::Lock& js, + kj::StringPtr type, + jsg::Optional> handler); + + // Registers an internal listener occupying a normal position in the listener list. The + // identity may later be passed to removeEventListener() to deactivate it. + void addEventHandlerListener(jsg::Lock& js, + kj::StringPtr type, + jsg::HashableV8Ref identity, + HandlerFunction callback); private: - // RAII-style listener that can be attached to an EventTarget. - class NativeHandler { - public: - using Signature = void(jsg::Ref); - NativeHandler(jsg::Lock& js, - EventTarget& target, - kj::String type, - jsg::Function func, - bool once = false); - ~NativeHandler() noexcept(false); - KJ_DISALLOW_COPY_AND_MOVE(NativeHandler); - - void operator()(jsg::Lock& js, jsg::Ref event); - - uint hashCode() const; - - void visitForGc(jsg::GcVisitor& visitor); - - private: - void detach(); - - kj::String type; - struct State { - // target's destructor will null out `state`, so this is OK to be a bare reference. - EventTarget& target; - - jsg::Function func; - }; - - kj::Maybe state; - bool once; - - friend class EventTarget; - }; - - void addNativeListener(jsg::Lock& js, NativeHandler& handler); - bool removeNativeListener(NativeHandler& handler); - struct EventHandler { - struct JavaScriptHandler { - jsg::HashableV8Ref identity; - HandlerFunction callback; - - // If the event handler is registered with an AbortSignal, then the abortHandler points - // at the NativeHandler representing that registration, so that if this object is GC'ed before - // the AbortSignal is signalled, we unregister ourselves from listening on it. Note that - // this is Own for the same reason newNativeHandler() returns Own: We are not - // supposed to do anything with this except drop it. - kj::Maybe> abortHandler; - - void visitForGc(jsg::GcVisitor& visitor) { - visitor.visit(identity, callback); - - // Note that we intentionally do NOT visit `abortHandler`. This is because the JS handles - // held by `abortHandler` are not ever accessed by this path. Instead, they are accessed - // by the AbortSignal, if and when it fires. So it is the AbortSignal's responsibility to - // visit the NativeHandler's content. - } - - kj::StringPtr jsgGetMemoryName() const { - return "JavaScriptHandler"_kjc; - } - size_t jsgGetMemorySelfSize() const; - void jsgGetMemoryInfo(jsg::MemoryTracker& tracker) const; - }; - - struct NativeHandlerRef { - NativeHandler& handler; - }; - - // An EventHandler can be backed by either a JavaScript Handler (which is either a - // function or an object) or a native handler. The insertion order matters here so - // we maintain a single table. - using Handler = kj::OneOf; - - Handler handler; + // The listener's identity (the function or object passed to addEventListener, or a + // synthesized object for internally-registered listeners), used for removeEventListener + // matching. + jsg::HashableV8Ref identity; + HandlerFunction callback; // When once is true, the handler will be removed after it is invoked one time. bool once = false; - EventHandler(Handler handler, bool once): handler(kj::mv(handler)), once(once) {} - KJ_DISALLOW_COPY_AND_MOVE(EventHandler); + // If the event handler is registered with an AbortSignal (the {signal} option), this + // holds the RAII registration for the signal's abort algorithm that removes this + // listener, so that if this entry goes away before the signal aborts, the algorithm is + // unregistered. The handle is opaque: the only thing to do with it is drop it. + kj::Maybe> abortHandler; + + void visitForGc(jsg::GcVisitor& visitor) { + visitor.visit(identity, callback); + + // Note that we intentionally do NOT visit `abortHandler`. It holds no JS references + // of its own; the algorithm it registers is owned — and GC-visited — by the + // AbortSignal it was registered with. + } kj::StringPtr jsgGetMemoryName() const { return "EventHandler"_kjc; @@ -517,16 +506,9 @@ class EventTarget: public jsg::Object { }; struct EventHandlerHashCallbacks { - const EventHandler::Handler& keyForRow(const kj::Own& row) const; + const jsg::HashableV8Ref& keyForRow(const kj::Own& row) const; bool matches(const kj::Own& a, const jsg::HashableV8Ref& b) const; - bool matches(const kj::Own& a, const NativeHandler& b) const; - bool matches(const kj::Own& a, const EventHandler::NativeHandlerRef& b) const; - bool matches(const kj::Own& a, const EventHandler::Handler& b) const; uint hashCode(const jsg::HashableV8Ref& obj) const; - uint hashCode(const NativeHandler& handler) const; - uint hashCode(const EventHandler::NativeHandlerRef& handler) const; - uint hashCode(const EventHandler::JavaScriptHandler& handler) const; - uint hashCode(const EventHandler::Handler& handler) const; }; struct EventHandlerSet { @@ -550,7 +532,35 @@ class EventTarget: public jsg::Object { kj::HashMap typeMap; - kj::Maybe maybeListenerCallback; + // State for one managed on event handler IDL attribute (HTML: an "event handler"). + struct EventHandlerAttribute { + struct Handler { + // The exact value assigned, returned by the getter. + jsg::JsRef value; + // The invocable form, present iff the assigned value was callable. A non-callable + // object is retained as the attribute value but never invoked. + kj::Maybe fn; + }; + kj::Maybe handler; + + // While activated, the identity of the trampoline listener entry occupying the + // attribute's position in the listener list. + kj::Maybe> listenerIdentity; + }; + + // Keyed by event type. An entry's presence marks on as managed: dispatch must not + // additionally consult the legacy on property reflection for that type, which would + // fire the handler twice. + kj::HashMap eventHandlerAttributes; + + bool managesEventHandlerAttribute(kj::StringPtr type) const { + return eventHandlerAttributes.find(type) != kj::none; + } + + // HTML "activate an event handler": registers the trampoline listener if it is not already + // registered (an already-active handler keeps its position across reassignment). + void activateEventHandlerAttribute( + jsg::Lock& js, kj::StringPtr type, EventHandlerAttribute& attribute); struct Flags { // When using module syntax, the "fetch", "scheduled", "trace", etc. @@ -565,8 +575,6 @@ class EventTarget: public jsg::Object { Flags flags; void visitForGc(jsg::GcVisitor& visitor); - - friend class NativeHandler; }; // An implementation of the Web Platform Standard AbortSignal API @@ -580,7 +588,7 @@ class AbortSignal final: public EventTarget { jsg::Optional> maybeReason = kj::none, Flag flag = Flag::NONE); - using PendingReason = ExternalPusherImpl::PendingAbortReason; + using PendingReason = ExternalPusherImpl::PendingAbortReasonBox; // The AbortSignal explicitly does not expose a constructor(). It is // illegal for user code to create an AbortSignal directly. @@ -609,23 +617,20 @@ class AbortSignal final: public EventTarget { void triggerAbort( jsg::Lock& js, jsg::Optional> maybeReason); - static jsg::Ref any(jsg::Lock& js, - kj::Array> signals, - const jsg::TypeHandler& handler, - const jsg::TypeHandler>& eventTargetHandler); + // Implements the spec's "create a dependent abort signal": returns a signal that aborts + // when any of the given signals abort, carrying the first aborter's reason. + static jsg::Ref any(jsg::Lock& js, kj::Array> signals); - // While AbortSignal extends EventTarget, and our EventTarget implementation will - // automatically support onabort being set as an own property, the spec defines - // onabort as a prototype property on the AbortSignal prototype. Therefore, we - // need to explicitly set it as a prototype property here. + // The onabort event handler IDL attribute (see EventTarget::setEventHandlerAttribute). kj::Maybe getOnAbort(jsg::Lock& js); - void setOnAbort(jsg::Lock& js, jsg::Optional handler); + void setOnAbort( + jsg::Lock& js, jsg::Optional> handler); - void addEventListener(jsg::Lock& js, - kj::String type, - jsg::Identified handler, - jsg::Optional maybeOptions, - const jsg::TypeHandler>& eventTargetHandler); + // Arms the RPC abort subscription whenever an 'abort' listener is registered — via + // addEventListener() or the onabort trampoline's activation. Registrations for other + // event types must not arm it: the subscription's pending awaitIo blocks hibernation in + // actors. + void listenerCountChanged(jsg::Lock& js, kj::StringPtr type, size_t count) override; JSG_RESOURCE_TYPE(AbortSignal, CompatibilityFlags::Reader flags) { JSG_INHERIT(EventTarget); @@ -644,17 +649,34 @@ class AbortSignal final: public EventTarget { if (flags.getWorkerdExperimental()) { JSG_METHOD(skipReleaseForTest); - JSG_TS_OVERRIDE({ skipReleaseForTest: never }); + JSG_METHOD(getNativeRegistrationCountForTest); + JSG_TS_OVERRIDE({ + skipReleaseForTest: never; + getNativeRegistrationCountForTest: never; + }); } } - // Allows this AbortSignal to also serve as a kj::Canceler + // Allows this AbortSignal to also serve as a kj::Canceler: the returned promise is + // canceled (rejected with a kj::Exception derived from the abort reason) if this signal + // is aborted. If the signal is ALREADY aborted, the returned promise is immediately + // rejected the same way, indistinguishable from an abort arriving right after wrapping. + // The cancellation runs in the calling IoContext; if the abort is triggered from a + // different request (or outside any request), it is delivered to the calling context the + // next time it runs. Requires an active IoContext. template kj::Promise wrap(jsg::Lock& js, kj::Promise promise) { - subscribeToRpcAbort(js); + if (getNeverAborts()) { + // This signal can never abort, so there is nothing to hook up. + return kj::mv(promise); + } - JSG_REQUIRE(!canceler->isCanceled(), TypeError, "The AbortSignal has already been triggered"); - return canceler->wrap(kj::mv(promise)); + // The wrapped promise carries the Cancellation — the (sole-owner) canceler and its + // registration, whose declaration order guarantees the registration unhooks before the + // canceler dies. + auto cancellation = newCanceler(js); + auto wrapped = cancellation.canceler->wrap(kj::mv(promise)); + return wrapped.attach(kj::mv(cancellation)); } template @@ -667,13 +689,65 @@ class AbortSignal final: public EventTarget { } } - RefcountedCanceler& getCanceler(); + // A canceler hooked up to this signal, plus the RAII registration keeping the hook alive. + // Returned by newCanceler() for native consumers that need more than promise wrapping + // (e.g. ReleasingCanceler::Listener callbacks). + struct Cancellation { + // Sole owner of the canceler. The signal's registration reaches it only by reference. + kj::Own canceler; + + // Keeps the canceler hooked to the signal; dropping it (from any thread) unhooks. + // + // WARNING: The registration's reference to the canceler is valid only while this handle + // is registered, so the holder MUST destroy this handle before (or together with, but + // ordered before) the canceler — i.e. declare it after the canceler member — and must + // keep both on the creating request's thread, as consumer objects owned by the request + // naturally are. + kj::Own registration; + }; + + // Creates a new canceler that is canceled — with a kj::Exception derived from the abort + // reason — when this signal aborts, following the same ownership and cross-request rules + // as wrap(). If the signal is already aborted (or can never abort), the returned canceler + // is pre-canceled (or inert) and no registration is made. Requires an active IoContext. + Cancellation newCanceler(jsg::Lock& js); + + // Registers a native callback to be invoked with the abort exception if/when this signal + // aborts. The callback runs under the isolate lock in the IoContext that is current at + // registration time: synchronously when the abort is triggered within that context, + // otherwise delivered on that context's next turn, and dropped entirely (never invoked) + // once that context has been destroyed. If the signal can never abort, the callback is + // never invoked and no registration is made. + // + // Dropping the returned handle (safe from any thread) unregisters the callback: once the + // handle is destroyed, the callback is guaranteed to never (again) be invoked, so it may + // capture references whose validity the holder ties to the handle's lifetime (see + // Cancellation::registration). Note that dropping the handle destroys the callback — and + // with it whatever the callback captured — on the dropping thread, so the captures + // themselves must be safe to destroy from any thread for the any-thread claim to hold. + // + // Requires an active IoContext. The caller is expected to have checked getAborted() first. + kj::Own addAbortAction( + jsg::Lock& js, kj::Function action); + + // Implements the DOM spec's "add an algorithm to signal's abort algorithms": registers a + // JS-heap callback that runs under the isolate lock, in whichever context triggers the + // abort, before the 'abort' event is dispatched — exactly once. Unlike addAbortAction(), + // no IoContext is required or captured, so the algorithm must only touch JS-heap state. + // Algorithms never run for synthetic dispatchEvent('abort') calls; only a real abort runs + // them (and then empties the list, per spec). + // + // Dropping the returned handle unregisters the algorithm; the handle holds only a weak + // reference to this signal and must be dropped under the isolate lock (it is expected to + // be held by JS-heap objects). The caller is expected to have checked getAborted() first: + // algorithms are never invoked retroactively. + kj::Own addAbortAlgorithm(jsg::Lock& js, jsg::Function algorithm); void visitForMemoryInfo(jsg::MemoryTracker& tracker) const { EventTarget::visitForMemoryInfo(tracker); - tracker.trackInlineFieldWithSize( - "IoOwn", sizeof(IoOwn)); tracker.trackField("reason", reason); + tracker.trackFieldWithSize( + "nativeRegistrations", nativeRegistrations.size() * sizeof(kj::Arc)); } void serialize(jsg::Lock& js, jsg::Serializer& serializer); @@ -682,6 +756,11 @@ class AbortSignal final: public EventTarget { // signal, this method will tell every rpcClient to skip this step before destruction. void skipReleaseForTest(); + // Test-only introspection: the number of native registration cells (live or empty awaiting + // a sweep) plus RPC registrations currently held by this signal. Lets tests assert that + // completed registrations are reclaimed rather than accumulating on long-lived signals. + int getNativeRegistrationCountForTest(); + static jsg::Ref deserialize( jsg::Lock& js, rpc::SerializationTag tag, jsg::Deserializer& deserializer); @@ -694,11 +773,91 @@ class AbortSignal final: public EventTarget { bool isIgnoredForSubrequests(jsg::Lock& js) const; private: - IoOwn canceler; Flag flag; + // Set iff this signal has been aborted; the source of truth for getAborted(). Also the + // exception native cancellations reject with. Plain data, safe on the JS heap: no + // IoContext is required to create or read a signal's abort state. + kj::Maybe maybeAbortException; + kj::Maybe> reason; - kj::Maybe> onAbortHandler; + + // One native abort action, shared between this signal and one consumer. The action is + // invoked at most once, only ever in its owning IoContext (synchronously if the abort is + // triggered there; otherwise on that context's next turn), always under the isolate lock, + // and never again after the consumer's RAII handle clears the slot. Because the handle is + // held by (or attached to) objects the owning request destroys, an IoContext teardown + // reclaims the action without ever touching the signal; the signal side retains only this + // trivial shell until swept. + // + // The action slot is taken under the mutex and invoked after unlocking. A cross-context + // abort does not take the slot; it schedules a task in the owning context that re-takes it + // on arrival — so a consumer that goes away in the meantime reliably turns the delivery + // into a no-op. + struct RegistrationCell final: public kj::AtomicRefcounted { + RegistrationCell( + IoCrossContextExecutor executor, kj::Function fn) + : executor(kj::mv(executor)) { + *action.lockExclusive() = kj::mv(fn); + } + + // Routes the action into the owning IoContext and answers "is that context current / + // still alive?". Immutable, so it is also usable for sweeping after the slot is cleared. + const IoCrossContextExecutor executor; + + kj::MutexGuarded>> action; + }; + + // Cells are appended on registration and taken wholesale when the signal aborts. Cells + // whose action has been cleared (consumer done, or its IoContext torn down) or whose + // owning context is gone are swept on the next registration; this bounds growth for + // long-lived signals used across many requests. Holds no JS heap references (weak refs at + // most), so no GC visitation is needed. + kj::Vector> nativeRegistrations; + + // Registers an abort action that cancels `canceler` with the abort exception when this + // signal aborts. The reference remains valid because the returned RAII handle guarantees + // the action never runs after the handle is destroyed, and the holder destroys the handle + // before the canceler (see Cancellation::registration). + kj::Own registerPendingCancellation(jsg::Lock& js, ReleasingCanceler& canceler); + + // The spec's "abort algorithms": insertion-ordered, run and then emptied by triggerAbort() + // before the 'abort' event is dispatched. Unlike the native registration cells, these hold + // JS-heap references and are therefore GC-visited. + struct AbortAlgorithm { + uint64_t token; + jsg::Function fn; + }; + kj::Vector abortAlgorithms; + uint64_t nextAbortAlgorithmToken = 0; + void removeAbortAlgorithm(uint64_t token); + + // Spec: "dependent" — true for signals created by AbortSignal.any(). + bool dependent = false; + + // Spec: "dependent signals" — signals created by AbortSignal.any() for which this signal + // is a source. Strong and GC-visited: a dependent must stay reachable as long as any of + // its sources could still abort it (V8 collects the cycle once neither side is otherwise + // reachable). Emptied when this signal aborts; a dependent that aborts first unlinks + // itself from its remaining sources via severSources(). + kj::Vector> dependentSignals; + + // Spec: "source signals" — the signals this dependent signal depends on. Weak: used only + // for AbortSignal.any()'s flattening rule (a dependent passed to any() contributes its + // sources, never itself, so dependency chains never form) and for severSources(). + kj::Vector> sourceSignals; + + // Records the abort reason and exception (spec "signal abort" step 2, also applied to + // dependents in steps 3-4 before any abort steps run). + void setAbortState(jsg::Lock& js, jsg::Optional> reason); + + // Spec "run the abort steps": abort algorithms, then workerd's native registrations (cells + // and RPC clones), then the 'abort' event. Requires setAbortState() to have run. + void runAbortSteps(jsg::Lock& js); + + // Removes this (aborted) dependent signal from any remaining sources so they no longer + // keep it alive or attempt to re-abort it. + void severSources(jsg::Lock& js); static kj::Exception abortException( jsg::Lock& js, const jsg::Optional>& reason); @@ -710,24 +869,43 @@ class AbortSignal final: public EventTarget { // ------------------------------------------------------------- // RPC client functionality. Used if this signal was serialized. - // A collection of rpcClients, which will be notified if this signal is triggered and when this - // signal is destroyed. - kj::Vector> rpcClients; + // One serialized clone of this signal, to be notified when the signal is triggered. The + // client is owned by the IoContext in which the signal was serialized — a signal shared + // across requests may hold registrations from several — and abort delivery is routed into + // that context like a native registration, re-taking the slot on arrival. There is no + // consumer-side RAII handle: the slot is reclaimed when the signal aborts, when a sweep + // finds the owning context destroyed, or when the signal itself is destroyed (either way + // the client's own destructor tells the peer that no abort is coming). + struct RpcRegistration final: public kj::AtomicRefcounted { + RpcRegistration(IoCrossContextExecutor executor, IoOwn client) + : executor(kj::mv(executor)) { + *this->client.lockExclusive() = kj::mv(client); + } - // Trigger an abort on all associated clients - kj::Promise sendToRpc(kj::Array&& reason); + const IoCrossContextExecutor executor; + kj::MutexGuarded>> client; + }; + kj::Vector> rpcRegistrations; // --------------------------------------------------------------- // RPC server functionality. Used if this signal was deserialized. + // Identifies the IoContext that deserialized this signal, which owns rpcAbortPromise + // below. Only that context can arm the RPC subscription (subscribeToRpcAbort); arming + // requested from any other context is routed here via tryExecute(), or dropped if the + // receiving context is already gone. + kj::Maybe rpcReceiverContext; + bool isRpcReceiverContextCurrent(); + // A promise that is fulfilled if an abort() message is received over RPC. kj::Maybe>> rpcAbortPromise; - // A refcounted object used to receive a serialized abort reason. - // The abort reason is required in asynchronous event handlers as well as synchronous methods - // like getReason(). As a result, we can't pass the abort reason in the above promise, and both - // sync and async methods will need to check this value. - kj::Maybe> pendingReason; + // The box through which a serialized abort reason arrives. The abort reason is required in + // asynchronous event handlers as well as synchronous methods like getReason(), so both sync + // and async paths check this value. It is written by the receiving request's RPC machinery + // but readable — under its mutex — from any context, so a deserialized signal that has + // crossed request boundaries still reports getAborted()/getReason() correctly everywhere. + kj::Maybe> pendingReason; // Synchronously check if an abort reason was sent over RPC bool hasPendingReason(); diff --git a/src/workerd/api/container.c++ b/src/workerd/api/container.c++ index 51a5c906b07..3f5c5302c09 100644 --- a/src/workerd/api/container.c++ +++ b/src/workerd/api/container.c++ @@ -148,21 +148,15 @@ ExecProcess::ExecProcess(jsg::Lock& js, KJ_IF_SOME(signal, abortSignal) { constexpr int kSigKill = 9; - auto& canceler = signal->getCanceler(); - // exec() calls throwIfAborted() before sending the RPC, but the signal can still fire while the // RPC is in flight, i.e. before this constructor runs in the RPC's continuation. If that - // happened, kill the freshly-started process immediately; there's no point registering a - // listener. - if (canceler.isCanceled()) { + // happened, kill the freshly-started process immediately; there's no point registering an + // abort action. + if (signal->getAborted(js)) { sendKill(kSigKill); } else { - // Hold a strong reference to the canceler so it outlives the AbortSignal's own IoOwn, then register - // a listener that kills the process when the signal is later triggered. - auto own = kj::addRef(canceler); - auto& ref = *own; - abortCanceler = ioContext.addObject(kj::mv(own)); - abortListener.emplace(ref, [self = JSG_THIS_WEAK(js)]() { + abortRegistration = signal->addAbortAction( + js, [self = JSG_THIS_WEAK(js)](jsg::Lock& js, const kj::Exception&) { KJ_IF_SOME(process, self.tryGet()) { process.sendKill(kSigKill); } diff --git a/src/workerd/api/container.h b/src/workerd/api/container.h index c7d03a4cb45..9b7ee672579 100644 --- a/src/workerd/api/container.h +++ b/src/workerd/api/container.h @@ -12,7 +12,6 @@ #include #include #include -#include #include namespace workerd::api { @@ -182,8 +181,9 @@ class ExecProcess: public jsg::Object { kj::Maybe resolvedExitCode; bool outputCalled = false; - kj::Maybe> abortCanceler; - kj::Maybe abortListener; + // Keeps the kill-on-abort action registered with the exec() options' AbortSignal for as + // long as this process object is alive. + kj::Maybe> abortRegistration; void visitForGc(jsg::GcVisitor& visitor) { visitor.visit(stdinStream, stdoutStream, stderrStream, exitCodePromise, exitCodePromiseCopy); diff --git a/src/workerd/api/events.c++ b/src/workerd/api/events.c++ index ad8a46c453c..744af834f33 100644 --- a/src/workerd/api/events.c++ +++ b/src/workerd/api/events.c++ @@ -13,54 +13,76 @@ constexpr kj::StringPtr kRejectionHandledEventName = "rejectionhandled"_kj; constexpr kj::StringPtr kUnhandledRejectionEventName = "unhandledrejection"_kj; } // namespace -OpenEvent::OpenEvent(): Event(kOpenEventName) {} +// Runtime-only; always trusted. +OpenEvent::OpenEvent(): Event(kOpenEventName, {}, Trusted::YES) {} MessageEvent::MessageEvent(jsg::Lock& js, const jsg::JsValue& data, kj::String lastEventId, kj::Maybe> source, - kj::Maybe urlForOrigin) - : Event(kMessageEventName), + kj::Maybe urlForOrigin, + Trusted trusted) + : Event(kMessageEventName, {}, trusted), data(jsg::JsRef(js, data)), lastEventId(kj::mv(lastEventId)), maybeSource(kj::mv(source)), - maybeOrigin(urlForOrigin.map([](auto& url) { return url.getOrigin(); })) {} + maybeOrigin(urlForOrigin.map([](auto& url) { return kj::str(url.getOrigin()); })) {} MessageEvent::MessageEvent(jsg::Lock& js, jsg::JsRef data, kj::String lastEventId, kj::Maybe> source, - kj::Maybe urlForOrigin) - : Event(kMessageEventName), + kj::Maybe urlForOrigin, + Trusted trusted) + : Event(kMessageEventName, {}, trusted), data(kj::mv(data)), lastEventId(kj::mv(lastEventId)), maybeSource(kj::mv(source)), - maybeOrigin(urlForOrigin.map([](auto& url) { return url.getOrigin(); })) {} + maybeOrigin(urlForOrigin.map([](auto& url) { return kj::str(url.getOrigin()); })) {} MessageEvent::MessageEvent(jsg::Lock& js, kj::String type, const jsg::JsValue& data, kj::String lastEventId, kj::Maybe> source, - kj::Maybe urlForOrigin) - : Event(kj::mv(type)), + kj::Maybe urlForOrigin, + Trusted trusted) + : Event(kj::mv(type), {}, trusted), data(jsg::JsRef(js, kj::mv(data))), lastEventId(kj::mv(lastEventId)), maybeSource(kj::mv(source)), - maybeOrigin(urlForOrigin.map([](auto& url) { return url.getOrigin(); })) {} + maybeOrigin(urlForOrigin.map([](auto& url) { return kj::str(url.getOrigin()); })) {} MessageEvent::MessageEvent(jsg::Lock& js, kj::String type, kj::OneOf, jsg::Ref> data, kj::String lastEventId, kj::Maybe> source, - kj::Maybe urlForOrigin) - : Event(kj::mv(type)), + kj::Maybe urlForOrigin, + Trusted trusted) + : Event(kj::mv(type), {}, trusted), data(kj::mv(data)), lastEventId(kj::mv(lastEventId)), maybeSource(kj::mv(source)), - maybeOrigin(urlForOrigin.map([](auto& url) { return url.getOrigin(); })) {} + maybeOrigin(urlForOrigin.map([](auto& url) { return kj::str(url.getOrigin()); })) {} + +MessageEvent::MessageEvent(jsg::Lock& js, kj::String type, Initializer initializer) + : Event(kj::mv(type), + Event::Init{ + .bubbles = initializer.bubbles, + .cancelable = initializer.cancelable, + .composed = initializer.composed, + }), + data(kj::mv(initializer.data).orDefault([&] { return jsg::JsRef(js, js.null()); })), + lastEventId(kj::mv(initializer.lastEventId).orDefault(kj::String())), + maybeSource(kj::mv(initializer.source)), + // Per the spec, origin defaults to the empty string for user-constructed events. + maybeOrigin(kj::mv(initializer.origin) + .map([](jsg::USVString&& origin) -> kj::String { return kj::mv(origin); }) + .orDefault(kj::String())), + ports( + kj::mv(initializer.ports).orDefault([] { return kj::Array>(); })) {} jsg::Ref MessageEvent::constructor( - jsg::Lock& js, kj::String type, Initializer initializer) { - return js.alloc(js, kj::mv(type), kj::mv(initializer.data)); + jsg::Lock& js, kj::String type, jsg::Optional initializer) { + return js.alloc(js, kj::mv(type), kj::mv(initializer).orDefault({})); } kj::OneOf> MessageEvent::getData(jsg::Lock& js) { @@ -75,8 +97,8 @@ kj::OneOf> MessageEvent::getData(jsg::Lock& js) { KJ_UNREACHABLE; } -kj::Maybe> MessageEvent::getOrigin() { - return maybeOrigin.map([](auto& a) -> kj::ArrayPtr { return a.asPtr(); }); +kj::Maybe MessageEvent::getOrigin() { + return maybeOrigin.map([](kj::String& origin) -> kj::StringPtr { return origin; }); } kj::StringPtr MessageEvent::getLastEventId() { @@ -89,11 +111,10 @@ kj::StringPtr MessageEvent::getLastEventId() { kj::Maybe> MessageEvent::getSource() { return maybeSource.map([](auto& port) mutable -> jsg::Ref { return port.addRef(); }); } -kj::ArrayPtr> MessageEvent::getPorts() { - // We don't support transferring MessagePorts in MessageEvent - // for now, so we return an empty array. Later we might support - // this. - return nullptr; +kj::Array> MessageEvent::getPorts() { + // The runtime never attaches ports (we don't support transferring MessagePorts), so this + // is empty except for user-constructed events that passed ports in their init. + return KJ_MAP(port, ports) -> jsg::Ref { return port.addRef(); }; } void MessageEvent::visitForMemoryInfo(jsg::MemoryTracker& tracker) const { @@ -106,6 +127,9 @@ void MessageEvent::visitForMemoryInfo(jsg::MemoryTracker& tracker) const { } } tracker.trackField("source", maybeSource); + for (auto& port: ports) { + tracker.trackField("port", port); + } } void MessageEvent::visitForGc(jsg::GcVisitor& visitor) { @@ -118,13 +142,22 @@ void MessageEvent::visitForGc(jsg::GcVisitor& visitor) { } } visitor.visit(maybeSource); + visitor.visitAll(ports); } // ====================================================================================== -ErrorEvent::ErrorEvent(ErrorEventInit init): Event(kDefaultErrorEventName), init(kj::mv(init)) {} +// Runtime-only (the JS constructor uses the (type, init) overload); always trusted. +ErrorEvent::ErrorEvent(ErrorEventInit init) + : Event(kDefaultErrorEventName, {}, Trusted::YES), + init(kj::mv(init)) {} ErrorEvent::ErrorEvent(kj::String type, ErrorEventInit init) - : Event(kj::mv(type)), + : Event(kj::mv(type), + Event::Init{ + .bubbles = init.bubbles, + .cancelable = init.cancelable, + .composed = init.composed, + }), init(kj::mv(init)) {} ErrorEvent::ErrorEvent(jsg::Lock& js, jsg::JsValue error) @@ -187,7 +220,8 @@ constexpr kj::StringPtr getPromiseRejectionEventName(v8::PromiseRejectEvent type PromiseRejectionEvent::PromiseRejectionEvent( v8::PromiseRejectEvent type, jsg::V8Ref promise, jsg::Value reason) - : Event(getPromiseRejectionEventName(type)), + // Runtime-only; always trusted. + : Event(getPromiseRejectionEventName(type), {}, Trusted::YES), promise(kj::mv(promise)), reason(kj::mv(reason)) {} diff --git a/src/workerd/api/events.h b/src/workerd/api/events.h index 2cac81f2aea..07f7956a561 100644 --- a/src/workerd/api/events.h +++ b/src/workerd/api/events.h @@ -16,42 +16,60 @@ class MessageEvent final: public Event { const jsg::JsValue& data, kj::String lastEventId = kj::String(), kj::Maybe> source = kj::none, - kj::Maybe urlForOrigin = kj::none); + kj::Maybe urlForOrigin = kj::none, + Trusted trusted = Trusted::NO); MessageEvent(jsg::Lock& js, jsg::JsRef data, kj::String lastEventId = kj::String(), kj::Maybe> source = kj::none, - kj::Maybe urlForOrigin = kj::none); + kj::Maybe urlForOrigin = kj::none, + Trusted trusted = Trusted::NO); MessageEvent(jsg::Lock& js, kj::String type, const jsg::JsValue& data, kj::String lastEventId = kj::String(), kj::Maybe> source = kj::none, - kj::Maybe urlForOrigin = kj::none); + kj::Maybe urlForOrigin = kj::none, + Trusted trusted = Trusted::NO); MessageEvent(jsg::Lock& js, kj::String type, kj::OneOf, jsg::Ref> data, kj::String lastEventId = kj::String(), kj::Maybe> source = kj::none, - kj::Maybe urlForOrigin = kj::none); + kj::Maybe urlForOrigin = kj::none, + Trusted trusted = Trusted::NO); + // The spec's MessageEventInit dictionary. Only `data` is ever meaningful to the runtime + // itself; the remaining members exist so that user-constructed events reflect the + // standard surface. struct Initializer { - jsg::JsRef data; - - JSG_STRUCT(data); + jsg::Optional bubbles; + jsg::Optional cancelable; + jsg::Optional composed; + jsg::Optional> data; + jsg::Optional origin; + jsg::Optional lastEventId; + jsg::Optional> source; + jsg::Optional>> ports; + + JSG_STRUCT(bubbles, cancelable, composed, data, origin, lastEventId, source, ports); JSG_STRUCT_TS_OVERRIDE(MessageEventInit { - data: ArrayBuffer | string; + data?: any; }); }; + + // For user-constructed events (the JS constructor path). + MessageEvent(jsg::Lock& js, kj::String type, Initializer initializer); + static jsg::Ref constructor( - jsg::Lock& js, kj::String type, Initializer initializer); + jsg::Lock& js, kj::String type, jsg::Optional initializer); kj::OneOf> getData(jsg::Lock& js); - kj::Maybe> getOrigin(); + kj::Maybe getOrigin(); kj::StringPtr getLastEventId(); @@ -60,7 +78,7 @@ class MessageEvent final: public Event { // support is MessagePort, return that if its set or null if not. kj::Maybe> getSource(); - kj::ArrayPtr> getPorts(); + kj::Array> getPorts(); JSG_RESOURCE_TYPE(MessageEvent) { JSG_INHERIT(Event); @@ -82,7 +100,11 @@ class MessageEvent final: public Event { kj::OneOf, jsg::Ref> data; kj::String lastEventId; kj::Maybe> maybeSource; - kj::Maybe> maybeOrigin; + kj::Maybe maybeOrigin; + + // The runtime never attaches ports (we do not support transferring MessagePorts); + // user-constructed events reflect the ports passed in their init. + kj::Array> ports; void visitForGc(jsg::GcVisitor& visitor); }; @@ -99,12 +121,15 @@ class OpenEvent final: public Event { class ErrorEvent final: public Event { public: struct ErrorEventInit { + jsg::Optional bubbles; + jsg::Optional cancelable; + jsg::Optional composed; jsg::Optional message; jsg::Optional filename; jsg::Optional lineno; jsg::Optional colno; jsg::Optional> error; - JSG_STRUCT(message, filename, lineno, colno, error); + JSG_STRUCT(bubbles, cancelable, composed, message, filename, lineno, colno, error); }; ErrorEvent(ErrorEventInit init); diff --git a/src/workerd/api/eventsource.c++ b/src/workerd/api/eventsource.c++ index 21e3e35687c..27d1b023189 100644 --- a/src/workerd/api/eventsource.c++ +++ b/src/workerd/api/eventsource.c++ @@ -276,7 +276,8 @@ EventSource::EventSource(jsg::Lock& js) abortController(js.alloc(js)), readyState(State::CONNECTING) {} -void EventSource::notifyError(jsg::Lock& js, const jsg::JsValue& error, bool reconnecting) { +void EventSource::notifyError( + jsg::Lock& js, const jsg::JsValue& error, bool reconnecting, AlreadyReported alreadyReported) { if (readyState == State::CLOSED) return; // Abort the connection if it hasn't already been. This will be a non-op if the @@ -288,35 +289,56 @@ void EventSource::notifyError(jsg::Lock& js, const jsg::JsValue& error, bool rec else readyState = State::CONNECTING; - // Dispatch the error event. - dispatchEventImpl(js, js.alloc(js, error)); + // Dispatch the error event. Report-only: the EventSource is already errored out at this + // point, so a throwing 'error' listener has its exception reported but triggers no + // further fail-fast reaction. + dispatchEventImpl(js, js.alloc(js, error), + effectiveExceptionPolicy(js, DispatchExceptionPolicy::REPORT)); - // Log the error as an uncaught exception for debugging purposes. - IoContext::current().logUncaughtException(UncaughtExceptionSource::ASYNC_TASK, error); + if (alreadyReported == AlreadyReported::NO) { + // Log the error as an uncaught exception for debugging purposes. + IoContext::current().logUncaughtException(UncaughtExceptionSource::ASYNC_TASK, error); + } } void EventSource::notifyOpen(jsg::Lock& js) { if (readyState == State::CLOSED) return; readyState = State::OPEN; - dispatchEventImpl(js, js.alloc()); + auto result = dispatchEventImpl( + js, js.alloc(), effectiveExceptionPolicy(js, DispatchExceptionPolicy::REPORT)); + KJ_IF_SOME(exception, result.firstException) { + // An 'open' listener threw. Its exception was reported (and the remaining listeners + // still ran); fail fast by erroring out the EventSource. + notifyError(js, exception.getHandle(js), false, AlreadyReported::YES); + } } void EventSource::notifyMessages(jsg::Lock& js, kj::Array messages) { if (readyState == State::CLOSED) return; + auto policy = effectiveExceptionPolicy(js, DispatchExceptionPolicy::REPORT); + // Under PROPAGATE (spec_compliant_dispatch_exceptions disabled), the first throwing + // listener ends the dispatch and its exception lands in the catch handler below, which + // errors out the EventSource — rather than escaping into (and failing) the enclosing + // read-loop task with no 'error' event at all. js.tryCatch([&] { for (auto& message: messages) { auto data = kj::str(kj::delimited(kj::mv(message.data), "\n"_kjc)); if (data.size() == 0) continue; kj::String type = kj::mv(message.event).orDefault([]() { return kj::str("message"); }); - dispatchEventImpl(js, + auto result = dispatchEventImpl(js, js.alloc(js, kj::mv(type), js.str(data), kj::mv(message.id), - kj::none /** source **/, impl.map([](FetchImpl& i) -> jsg::Url& { return i.url; }))); + kj::none /** source **/, impl.map([](FetchImpl& i) -> jsg::Url& { return i.url; }), + Trusted::YES), + policy); + KJ_IF_SOME(exception, result.firstException) { + // A listener threw under REPORT. Its exception was reported (and the remaining + // listeners for this event still ran); fail fast: error out the EventSource and + // drop the remaining messages in this batch. + notifyError(js, exception.getHandle(js), false, AlreadyReported::YES); + return; + } } - }, [&](jsg::Value exception) { - // If we end up with an exception being thrown in one of the event handlers, we will - // stop trying to process the messages and instead just error the EventSource. - notifyError(js, jsg::JsValue(exception.getHandle(js))); - }); + }, [&](jsg::Value exception) { notifyError(js, jsg::JsValue(exception.getHandle(js))); }); } void EventSource::reconnect(jsg::Lock& js) { @@ -511,7 +533,7 @@ void EventSource::visitForGc(jsg::GcVisitor& visitor) { KJ_IF_SOME(i, impl) { visitor.visit(i.options.fetcher); } - visitor.visit(abortController, onopenValue, onmessageValue, onerrorValue); + visitor.visit(abortController); } void EventSource::visitForMemoryInfo(jsg::MemoryTracker& tracker) const { @@ -521,9 +543,6 @@ void EventSource::visitForMemoryInfo(jsg::MemoryTracker& tracker) const { } tracker.trackField("abortController", abortController); tracker.trackField("lastEventId", lastEventId); - tracker.trackField("onopen", onopenValue); - tracker.trackField("onmessage", onmessageValue); - tracker.trackField("onerror", onerrorValue); } } // namespace workerd::api diff --git a/src/workerd/api/eventsource.h b/src/workerd/api/eventsource.h index 70158abf21b..c0c84498122 100644 --- a/src/workerd/api/eventsource.h +++ b/src/workerd/api/eventsource.h @@ -9,9 +9,12 @@ #include #include +#include namespace workerd::api { +WD_STRONG_BOOL(AlreadyReported); + using kj::uint; class Fetcher; class ReadableStream; @@ -67,38 +70,28 @@ class EventSource: public EventTarget { // will cause the stream to be canceled. static jsg::Ref from(jsg::Lock& js, JsReadableStream stream); + // The onopen, onmessage, and onerror event handler IDL attributes + // (see EventTarget::setEventHandlerAttribute). kj::Maybe getOnOpen(jsg::Lock& js) { - return onopenValue.map( - [&](jsg::JsRef& ref) -> jsg::JsValue { return ref.getHandle(js); }); + return getEventHandlerAttribute(js, "open"_kj); } - void setOnOpen(jsg::Lock& js, jsg::JsValue value) { - if (!value.isObject() && !value.isFunction()) { - onopenValue = kj::none; - } else { - onopenValue = jsg::JsRef(js, value); - } + void setOnOpen( + jsg::Lock& js, jsg::Optional> handler) { + setEventHandlerAttribute(js, "open"_kj, kj::mv(handler)); } kj::Maybe getOnMessage(jsg::Lock& js) { - return onmessageValue.map( - [&](jsg::JsRef& ref) -> jsg::JsValue { return ref.getHandle(js); }); + return getEventHandlerAttribute(js, "message"_kj); } - void setOnMessage(jsg::Lock& js, jsg::JsValue value) { - if (!value.isObject() && !value.isFunction()) { - onmessageValue = kj::none; - } else { - onmessageValue = jsg::JsRef(js, value); - } + void setOnMessage( + jsg::Lock& js, jsg::Optional> handler) { + setEventHandlerAttribute(js, "message"_kj, kj::mv(handler)); } kj::Maybe getOnError(jsg::Lock& js) { - return onerrorValue.map( - [&](jsg::JsRef& ref) -> jsg::JsValue { return ref.getHandle(js); }); + return getEventHandlerAttribute(js, "error"_kj); } - void setOnError(jsg::Lock& js, jsg::JsValue value) { - if (!value.isObject() && !value.isFunction()) { - onerrorValue = kj::none; - } else { - onerrorValue = jsg::JsRef(js, value); - } + void setOnError( + jsg::Lock& js, jsg::Optional> handler) { + setEventHandlerAttribute(js, "error"_kj, kj::mv(handler)); } JSG_RESOURCE_TYPE(EventSource) { @@ -172,9 +165,6 @@ class EventSource: public EventTarget { // The EventSource spec defines onopen, onmessage, and onerror as prototype // properties on the class. - kj::Maybe> onopenValue; - kj::Maybe> onmessageValue; - kj::Maybe> onerrorValue; // The default reconnection wait time. This is fairly arbitrary and is left // entirely up to the implementation. The event stream can provide a new value. @@ -185,7 +175,13 @@ class EventSource: public EventTarget { kj::Duration reconnectionTime = DEFAULT_RECONNECTION_TIME; void notifyOpen(jsg::Lock& js); - void notifyError(jsg::Lock& js, const jsg::JsValue& error, bool reconnecting = false); + // AlreadyReported::YES indicates the error was already delivered to the global scope's + // report-an-exception machinery (a reported listener exception), so notifyError() must + // not log it again. + void notifyError(jsg::Lock& js, + const jsg::JsValue& error, + bool reconnecting = false, + AlreadyReported alreadyReported = AlreadyReported::NO); void notifyMessages(jsg::Lock& js, kj::Array messages); // The run() method handles the actual processing of the stream. diff --git a/src/workerd/api/global-scope.c++ b/src/workerd/api/global-scope.c++ index cfa1e59162d..92e4fac4d7c 100644 --- a/src/workerd/api/global-scope.c++ +++ b/src/workerd/api/global-scope.c++ @@ -394,7 +394,7 @@ kj::Promise> ServiceWorkerGlobalScope::request(kj::HttpMetho } } else { // Fire off the handlers. - useDefaultHandling = dispatchEventImpl(lock, event.addRef()); + useDefaultHandling = dispatchEventImpl(lock, event.addRef()).result; } if (useDefaultHandling) { @@ -1134,24 +1134,63 @@ void ServiceWorkerGlobalScope::reportError(jsg::Lock& js, jsg::JsValue error) { // Per the spec, we are going to first emit an error event on the global object. // If that event is not prevented, we will log the error to the console. Note // that we do not throw the error at all. + const auto logError = [&](const jsg::JsValue& error) { + // This helper must not throw: it is reached from dispatch paths with a no-throw contract + // (a REPORT-policy dispatch, and the re-entrancy branch below). Reading `stack` can run + // arbitrary user code — a getter or proxy trap — so a failure there falls back to the + // generic logging, which is side-effect-free (ToDetailString; no user code). + JSG_TRY(js) { + // If the value is an object that has a stack property, log that so we get + // the stack trace if it is an exception. + KJ_IF_SOME(obj, error.tryCast()) { + auto stack = obj.get(js, "stack"_kj); + if (!stack.isUndefined()) { + js.reportError(stack); + return; + } + } + // Otherwise just log the stringified value generically. + js.reportError(error); + } + JSG_CATCH(exception KJ_UNUSED) { + // Getting the stack property can throw an error if the accessor is + // overridden by user code, etc. We don't want to propagate that error + // because it violates the no-throw contract of this function, but we + // don't want to just swallow it either. Let's log so we can at least + // have a record of it happening at all. We dont want to log every case + // or spam sentry, so let's log periodically with NOSENTRY. + LOG_PERIODICALLY( + WARNING, "NOSENTRY Error while reporting error to console", exception.getHandle(js)); + js.reportError(error); + }; + }; + + // Per HTML's "report an exception" re-entrancy guard (the global's "in error reporting + // mode" flag): an exception reported while the 'error' event is being dispatched — e.g. + // an 'error' listener that itself throws, which the REPORT dispatch policy routes right + // back here — skips the event and goes straight to the console. Without this, a throwing + // 'error' listener would either propagate out of whatever REPORT dispatch triggered the + // report (violating its no-throw contract) or recurse indefinitely. + if (inErrorReportingMode) { + logError(error); + return; + } + inErrorReportingMode = true; + KJ_DEFER(inErrorReportingMode = false); + + // Technically speaking, the jsg::checks below can also trigger a throw, but + // these aren't triggering user code so it's unlikely unless we're in a fatal + // state. Just allow the error to propagate in these cases. auto message = v8::Exception::CreateMessage(js.v8Isolate, error); auto event = js.alloc(ErrorEvent::ErrorEventInit{.message = kj::str(message->Get()), .filename = kj::str(message->GetScriptResourceName()), .lineno = jsg::check(message->GetLineNumber(js.v8Context())), .colno = jsg::check(message->GetStartColumn(js.v8Context())), .error = jsg::JsRef(js, error)}); - if (dispatchEventImpl(js, kj::mv(event))) { - // If the value is an object that has a stack property, log that so we get - // the stack trace if it is an exception. - KJ_IF_SOME(obj, error.tryCast()) { - auto stack = obj.get(js, "stack"_kj); - if (!stack.isUndefined()) { - js.reportError(stack); - return; - } - } - // Otherwise just log the stringified value generically. - js.reportError(error); + if (dispatchEventImpl( + js, kj::mv(event), effectiveExceptionPolicy(js, DispatchExceptionPolicy::REPORT)) + .result) { + logError(error); } } diff --git a/src/workerd/api/global-scope.h b/src/workerd/api/global-scope.h index 926963caefe..cc5186ac82b 100644 --- a/src/workerd/api/global-scope.h +++ b/src/workerd/api/global-scope.h @@ -1178,6 +1178,11 @@ class ServiceWorkerGlobalScope: public WorkerGlobalScope { jsg::UnhandledRejectionHandler unhandledRejections; kj::Maybe> processValue; kj::Maybe> bufferValue; + + // HTML's "in error reporting mode" flag: set while reportError() is dispatching the + // 'error' event, so that a nested report (e.g. from a throwing 'error' listener) logs + // directly instead of recursing. + bool inErrorReportingMode = false; kj::Maybe> defaultFetcher; kj::HashMap connectOverrides; kj::HashMap dnsOverrides; diff --git a/src/workerd/api/http.c++ b/src/workerd/api/http.c++ index 6623ae584a9..5a189d2e6f7 100644 --- a/src/workerd/api/http.c++ +++ b/src/workerd/api/http.c++ @@ -1603,7 +1603,9 @@ jsg::Promise> fetchImplNoOutputLock(jsg::Lock& js, if (s->getAborted(js)) { return js.rejectedPromise>(s->getReason(js)); } - webSocket = kj::refcounted(kj::mv(webSocket), s->getCanceler()); + auto cancellation = s->newCanceler(js); + webSocket = kj::refcounted(kj::mv(webSocket), + kj::mv(cancellation.canceler), kj::mv(cancellation.registration)); } return js.resolvedPromise(makeHttpResponse(js, jsRequest->getMethodEnum(), kj::mv(urlList), response.statusCode, response.statusText, *response.headers, @@ -1730,7 +1732,9 @@ jsg::Promise> handleHttpResponse(jsg::Lock& js, if (s->getAborted(js)) { return js.rejectedPromise>(s->getReason(js)); } - response.body = kj::refcounted(kj::mv(response.body), s->getCanceler()); + auto cancellation = s->newCanceler(js); + response.body = kj::refcounted( + kj::mv(response.body), kj::mv(cancellation.canceler), kj::mv(cancellation.registration)); } if (isRedirectStatusCode(response.statusCode) && diff --git a/src/workerd/api/messagechannel.c++ b/src/workerd/api/messagechannel.c++ index 695b170dc82..4c8380efe76 100644 --- a/src/workerd/api/messagechannel.c++ +++ b/src/workerd/api/messagechannel.c++ @@ -1,54 +1,73 @@ #include "messagechannel.h" +#include "blob.h" #include "events.h" +#include #include namespace workerd::api { -MessagePort::MessagePort(): state(Pending()) { - // We set a callback on the underlying EventTarget to be notified when - // a listener for the message event is added or removed. When there - // are no listeners, we move back to the Pending state, otherwise we - // will switch to the Started state if necessary. - setEventListenerCallback([&](jsg::Lock& js, kj::StringPtr name, size_t count) { - if (name == "message"_kj) { - KJ_SWITCH_ONEOF(state) { - KJ_CASE_ONEOF(pending, Pending) { - // If we are in the pending state, start the port if we have listeners. - // This is technically not spec compliant, but it is what Node.js - // supports. Specifically, adding a new message listener using the - // addEventListener method is *technically* not supposed to start - // the port but we're going to do what Node.js does. - if (count > 0 || onmessageValue != kj::none) { - start(js); - } - } - KJ_CASE_ONEOF(started, Started) { - // If we are in the started state, stop the port if there are no listeners. - if (count == 0 && onmessageValue == kj::none) { - state = Pending(); - } - } - KJ_CASE_ONEOF(_, Closed) { - // Nothing to do. We're already closed so we don't care. - } +MessagePort::MessagePort(): state(Pending()) {} + +// Tracks 'message' listener registrations — both addEventListener() listeners and the +// onmessage attribute's trampoline — to transition the port between states: the first +// listener starts the port (delivering any queued messages), and removing the last one +// returns it to pending (queueing messages again). Counting every listener is technically +// not spec compliant (per spec only assigning onmessage enables the message queue), but it +// is what Node.js does. +void MessagePort::listenerCountChanged(jsg::Lock& js, kj::StringPtr type, size_t count) { + if (type != "message"_kj) { + return; + } + KJ_SWITCH_ONEOF(state) { + KJ_CASE_ONEOF(pending, Pending) { + if (count > 0) { + start(js); + } + } + KJ_CASE_ONEOF(started, Started) { + if (count == 0) { + state = Pending(); } } - }); + KJ_CASE_ONEOF(_, Closed) { + // Closed is terminal: listener changes never restart the port. + } + } } void MessagePort::dispatchMessage(jsg::Lock& js, const jsg::JsValue& value) { - JSG_TRY(js) { - auto message = js.alloc(js, value, kj::String(), JSG_THIS); - dispatchEventImpl(js, kj::mv(message)); + auto policy = effectiveExceptionPolicy(js, DispatchExceptionPolicy::REPORT); + if (policy == DispatchExceptionPolicy::PROPAGATE) { + // Compat path (spec_compliant_dispatch_exceptions disabled): the first throwing + // listener ends the dispatch; the exception is swallowed and re-dispatched as a second + // 'message' event carrying the exception as its data. (The spec path below uses a + // 'messageerror' event instead; the 'message' type here is retained for compatibility.) + // If that second dispatch throws, the exception propagates: the delivery microtask + // fails. + JSG_TRY(js) { + dispatchEventImpl( + js, js.alloc(js, value, kj::String(), JSG_THIS, kj::none, Trusted::YES)); + } + JSG_CATCH(exception) { + dispatchEventImpl(js, + js.alloc(js, jsg::JsValue(exception.getHandle(js)), kj::String(), JSG_THIS, + kj::none, Trusted::YES)); + } + return; } - JSG_CATCH(exception) { - // There was an error dispatching the message event. - // We will dispatch a messageerror event instead. - auto message = - js.alloc(js, jsg::JsValue(exception.getHandle(js)), kj::String(), JSG_THIS); - dispatchEventImpl(js, kj::mv(message)); - // Now, if this dispatchEventImpl throws, we just blow up. Don't try to catch it. + + auto result = dispatchEventImpl(js, + js.alloc(js, value, kj::String(), JSG_THIS, kj::none, Trusted::YES), policy); + KJ_IF_SOME(exception, result.firstException) { + // A 'message' listener threw. Its exception was reported (and the remaining 'message' + // listeners still ran); additionally surface it as a 'messageerror' event on this port, + // carrying the exception as the event's data. The 'messageerror' dispatch itself is + // report-only: a throwing 'messageerror' listener triggers no further reaction. + dispatchEventImpl(js, + js.alloc(js, kj::str("messageerror"), exception.addRef(js), kj::String(), + JSG_THIS, kj::none, Trusted::YES), + DispatchExceptionPolicy::REPORT); } } @@ -163,7 +182,7 @@ void MessagePort::close(jsg::Lock& js) { } other = kj::none; } - auto closeEvent = js.alloc(name, Event::Init{}); + auto closeEvent = js.alloc(name, Event::Init{}, Trusted::YES); dispatchEventImpl(js, kj::mv(closeEvent)); } @@ -192,23 +211,14 @@ void MessagePort::start(jsg::Lock& js) { } kj::Maybe MessagePort::getOnMessage(jsg::Lock& js) { - return onmessageValue.map( - [&](jsg::JsRef& ref) -> jsg::JsValue { return ref.getHandle(js); }); + return getEventHandlerAttribute(js, "message"_kj); } -void MessagePort::setOnMessage(jsg::Lock& js, jsg::JsValue value) { - if (!value.isObject() && !value.isFunction()) { - onmessageValue = kj::none; - // If we have no handlers and no onmessage ... - if (getHandlerCount("message"_kj) == 0 && onmessageValue == kj::none) { - // ...Put the port back into a pending state where messages - // will be enqueued until another listener is attached. - state = Pending(); - } - } else { - onmessageValue = jsg::JsRef(js, value); - start(js); - } +void MessagePort::setOnMessage( + jsg::Lock& js, jsg::Optional> handler) { + // The attribute's trampoline registration and removal flow through + // listenerCountChanged(), which starts and stops the port; nothing else to do here. + setEventHandlerAttribute(js, "message"_kj, kj::mv(handler)); } jsg::Ref MessageChannel::constructor(jsg::Lock& js) { diff --git a/src/workerd/api/messagechannel.h b/src/workerd/api/messagechannel.h index 88c0be2b635..8bc901b8d5e 100644 --- a/src/workerd/api/messagechannel.h +++ b/src/workerd/api/messagechannel.h @@ -21,18 +21,22 @@ namespace workerd::api { // list semantics, but we do validate the transfer list input to an extent. // - It does not support serialization/deserialization. It's not possible to // send a MessagePort anywhere currently. -// - The `messageerror` event is only partially implemented. Currently, if a -// message data cannot be serialized/deserialized it will throw an error -// synchronously when posted rather than dispatching the `messageerror` event -// on the receiving port, this is just easiest to implement for now and makes -// the most sense for our current use case since the MessagePort only ever -// passes messages around within the same isolate (that is, we're not sending -// the serialized data off anywhere, we're just cloning it and dispatching it.) +// - The `messageerror` event diverges from the spec. If message data cannot be +// serialized it throws synchronously from postMessage() rather than dispatching +// `messageerror` on the receiving port; this is easiest for now and makes the +// most sense for our current use case since the MessagePort only ever passes +// messages around within the same isolate (that is, we're not sending the +// serialized data off anywhere, we're just cloning it and dispatching it.) +// Instead, a throwing 'message' listener — whose exception is reported per +// spec — additionally dispatches a `messageerror` event on this port carrying +// the exception as its data, which the spec does not do. // - We intentionally do not implement the "port message queue" semantics exactly -// as they are described in the spec. When a MessagePort has an onmessage listener, -// the message delivery is flowing, when there is no onmessage listener, the -// messages are queued up until the port is started. Because we are storing -// these as JS values, we don't worry about extra memory accounting for the queue. +// as they are described in the spec. While the port has any 'message' listener — +// whether assigned to onmessage or added with addEventListener(), which per spec +// would not enable the queue but does in Node.js — message delivery is flowing; +// when the last one is removed, messages are queued until another is attached or +// start() is called. Because we are storing these as JS values, we don't worry +// about extra memory accounting for the queue. // - We do not emit the close event on entangled ports when one of them is GC'd. // - We do not check to see if a MessagePort is entangled with another when we // call entangle because there's only one way to entangle them currently and @@ -78,13 +82,11 @@ class MessagePort final: public EventTarget { void close(jsg::Lock& js); void start(jsg::Lock& js); - // Support the onmessage getter and setter. Per the spec, when - // onmessage is set, the MessagePort is automatically started, - // but when addEventListener is set, start must be called - // separately. That's a kind of a weird rule but ok. To support - // that we need to define an onmessage getter/setter pair. + // The onmessage event handler IDL attribute + // (see EventTarget::setEventHandlerAttribute). kj::Maybe getOnMessage(jsg::Lock& js); - void setOnMessage(jsg::Lock& js, jsg::JsValue value); + void setOnMessage( + jsg::Lock& js, jsg::Optional> handler); JSG_RESOURCE_TYPE(MessagePort) { JSG_INHERIT(EventTarget); @@ -129,13 +131,13 @@ class MessagePort final: public EventTarget { // To keep them both alive, maintain strong references to both // ports! kj::Maybe> other; - kj::Maybe> onmessageValue; + + void listenerCountChanged(jsg::Lock& js, kj::StringPtr type, size_t count) override; void visitForGc(jsg::GcVisitor& visitor) { KJ_IF_SOME(pending, state.tryGet()) { visitor.visitAll(pending); } - visitor.visit(onmessageValue); } }; diff --git a/src/workerd/api/tests/BUILD.bazel b/src/workerd/api/tests/BUILD.bazel index b20839aa32b..f91f06a2de0 100644 --- a/src/workerd/api/tests/BUILD.bazel +++ b/src/workerd/api/tests/BUILD.bazel @@ -531,6 +531,12 @@ wd_test( data = ["eventsource-test.js"], ) +wd_test( + src = "legacy-dispatch-exceptions-test.wd-test", + args = ["--experimental"], + data = ["legacy-dispatch-exceptions-test.js"], +) + wd_test( src = "form-data-legacy-test.wd-test", args = ["--experimental"], diff --git a/src/workerd/api/tests/abortsignal-test.js b/src/workerd/api/tests/abortsignal-test.js index 5e44af3430f..e5609322e59 100644 --- a/src/workerd/api/tests/abortsignal-test.js +++ b/src/workerd/api/tests/abortsignal-test.js @@ -1,7 +1,14 @@ // Copyright (c) 2023 Cloudflare, Inc. // Licensed under the Apache 2.0 license found in the LICENSE file or at: // https://opensource.org/licenses/Apache-2.0 -import { strictEqual, ok, throws, rejects } from 'node:assert'; +import { + strictEqual, + deepStrictEqual, + ok, + throws, + rejects, + match, +} from 'node:assert'; import { WorkerEntrypoint, RpcTarget } from 'cloudflare:workers'; // Test for the AbortSignal and AbortController standard Web API implementations. @@ -22,7 +29,18 @@ class WrappedAbortSignal extends RpcTarget { } } +// Creating AbortController/AbortSignal (and objects that allocate one, such as a Request's +// lazily-created signal) does not require an active IoContext: module-scope creation works. +// These are exercised by the globalScopeCreation and crossRequest* tests below. +const moduleScopeController = new AbortController(); +const moduleScopePreAborted = AbortSignal.abort('module-scope'); +const moduleScopeRequestSignal = new Request('http://example.org').signal; +const moduleScopeChurnController = new AbortController(); + let globalAbortController; +let globalWaitController; +let heldRpcSignal; +let heldRpcSignalObserved; export class RpcRemoteEnd extends WorkerEntrypoint { async echo(signal) { return signal; @@ -78,14 +96,89 @@ export class RpcRemoteEnd extends WorkerEntrypoint { if (globalAbortController === undefined) { globalAbortController = new AbortController(); await this.env.RpcRemoteEnd.echo(globalAbortController.signal); // send the signal over + return 'created'; } else { globalAbortController.abort(new Error('boom?')); + return { + aborted: globalAbortController.signal.aborted, + reason: globalAbortController.signal.reason.message, + }; } } async getWrappedSignal() { return new WrappedAbortSignal(); } + + // Starts a long native wait hooked to a module-scope signal. The wait's cancellation hook + // is owned by this request's IoContext; a later abort from a different request must be + // delivered into this context, rejecting the wait long before its timeout. + async startAbortableWait() { + globalWaitController = new AbortController(); + try { + await scheduler.wait(10_000, { signal: globalWaitController.signal }); + return 'completed'; + } catch (err) { + return `aborted:${err.message}`; + } + } + + // Aborts the wait started by startAbortableWait() from a different request's context. + async abortGlobalWait() { + globalWaitController.abort(new Error('cross-request')); + return globalWaitController.signal.aborted; + } + + // Waits on a native timer wrapped with a signal received over RPC. The wrap's abort action + // is what arms the RPC abort subscription, so a remote abort must cancel the wait. + async waitOnReceivedSignal(signal) { + try { + await scheduler.wait(10_000, { signal }); + return 'completed'; + } catch (err) { + return `aborted:${err.message}`; + } + } + + // One short signal-wrapped wait against a module-scope controller. Each RPC call runs in + // its own request, so repeated calls register and release one native cancellation hook per + // request on the same long-lived signal. + async churnWait() { + await scheduler.wait(1, { signal: moduleScopeChurnController.signal }); + return 'ok'; + } + + async abortChurnController() { + moduleScopeChurnController.abort(new Error('churn-done')); + } + + // Deserializes a signal and parks this request — the signal's RPC receiver context — + // without registering any abort observer itself. Resolved by the 'abort' listener that + // listenOnHeldSignal() registers from a different request. The long timer keeps this + // request pending (a bare parked promise would trip the hang detector) and bounds the + // failure mode to a clean timeout. + async holdReceivedSignal(signal) { + heldRpcSignal = signal; + const { promise, resolve } = Promise.withResolvers(); + heldRpcSignalObserved = resolve; + return await Promise.race([ + promise, + scheduler.wait(10_000).then(() => 'timed-out'), + ]); + } + + // Runs in its own request: registers an 'abort' listener on the signal held by + // holdReceivedSignal()'s request. Arming the RPC abort subscription is routed into that + // request's context, which owns the underlying RPC promise. + async listenOnHeldSignal() { + while (heldRpcSignal === undefined) { + await scheduler.wait(10); + } + heldRpcSignal.addEventListener('abort', () => { + heldRpcSignalObserved(`aborted:${heldRpcSignal.reason.message}`); + }); + return heldRpcSignal.aborted; + } } export const abortcontroller = { @@ -525,20 +618,254 @@ export const rpcRequestSignal = { }, }; +export const abortAlgorithmOrdering = { + test() { + // The {signal} option registers an abort *algorithm*, which runs before any 'abort' + // listeners fire: by the time abort listeners run, a {signal}-registered listener is + // already removed, even if the abort listener was registered first. + const ac = new AbortController(); + const target = new EventTarget(); + let fired = false; + ac.signal.addEventListener('abort', () => { + target.dispatchEvent(new Event('foo')); + }); + target.addEventListener( + 'foo', + () => { + fired = true; + }, + { signal: ac.signal } + ); + ac.abort(); + strictEqual(fired, false); + // And it stays removed afterward. + target.dispatchEvent(new Event('foo')); + strictEqual(fired, false); + }, +}; + +export const syntheticAbortDispatch = { + test() { + // A synthetic dispatchEvent('abort') fires listeners but runs none of the internal + // abort plumbing: the signal does not become aborted, {signal}-registered listeners + // survive, and dependent signals do not abort. + const ac = new AbortController(); + const dependent = AbortSignal.any([ac.signal]); + const target = new EventTarget(); + let fooCount = 0; + let abortCount = 0; + target.addEventListener('foo', () => fooCount++, { signal: ac.signal }); + ac.signal.addEventListener('abort', () => abortCount++); + + ac.signal.dispatchEvent(new Event('abort')); + strictEqual(abortCount, 1); + strictEqual(ac.signal.aborted, false); + strictEqual(dependent.aborted, false); + target.dispatchEvent(new Event('foo')); + strictEqual(fooCount, 1); // the listener is still registered + + // A real abort still works after the synthetic one. + ac.abort(); + strictEqual(abortCount, 2); + strictEqual(ac.signal.aborted, true); + strictEqual(dependent.aborted, true); + target.dispatchEvent(new Event('foo')); + strictEqual(fooCount, 1); // now removed by the real abort's algorithm + }, +}; + +export const onabortPosition = { + test() { + // onabort occupies the position in the listener list where it was first activated, and + // reassignment keeps that position (HTML event handler semantics). + const ac = new AbortController(); + const order = []; + ac.signal.addEventListener('abort', () => order.push('L1')); + ac.signal.onabort = () => order.push('H-replaced'); + ac.signal.onabort = () => order.push('H'); + ac.signal.addEventListener('abort', () => order.push('L2')); + ac.abort(); + deepStrictEqual(order, ['L1', 'H', 'L2']); + }, +}; + +export const onabortReposition = { + test() { + // Deactivating (assigning null) and reassigning takes a fresh position. + const ac = new AbortController(); + const order = []; + ac.signal.onabort = () => order.push('H-deactivated'); + ac.signal.addEventListener('abort', () => order.push('L1')); + ac.signal.onabort = null; + ac.signal.onabort = () => order.push('H'); + ac.abort(); + deepStrictEqual(order, ['L1', 'H']); + }, +}; + +export const onabortNonCallable = { + test() { + // Per [LegacyTreatNonObjectAsNull]: a non-callable object is retained as the attribute + // value but never invoked; a non-object assignment is treated as null. + const ac = new AbortController(); + const obj = { + handleEvent() { + throw new Error('must not be called'); + }, + }; + ac.signal.onabort = obj; + strictEqual(ac.signal.onabort, obj); + ac.signal.onabort = 'nope'; + strictEqual(ac.signal.onabort, null); + ac.abort(); + }, +}; + +export const throwingAbortListener = { + test() { + // Per spec, "signal abort" cannot throw: a throwing listener's exception is reported to + // the global scope (via the cancelable 'error' event) and the remaining listeners run. + const ac = new AbortController(); + const order = []; + let reported = null; + const errorHandler = (ev) => { + reported = ev.error; + ev.preventDefault(); + }; + globalThis.addEventListener('error', errorHandler); + try { + ac.signal.addEventListener('abort', () => { + order.push('L1'); + throw new Error('boom'); + }); + ac.signal.onabort = () => order.push('H'); + ac.signal.addEventListener('abort', () => order.push('L2')); + ac.abort(); + } finally { + globalThis.removeEventListener('error', errorHandler); + } + deepStrictEqual(order, ['L1', 'H', 'L2']); + strictEqual(ac.signal.aborted, true); + strictEqual(reported?.message, 'boom'); + }, +}; + +export const globalScopeCreation = { + test() { + // The module-scope objects above were created during module evaluation, with no active + // IoContext. Verify they are fully functional. + strictEqual(moduleScopePreAborted.aborted, true); + strictEqual(moduleScopePreAborted.reason, 'module-scope'); + strictEqual(moduleScopeRequestSignal.aborted, false); + + strictEqual(moduleScopeController.signal.aborted, false); + let fired = false; + moduleScopeController.signal.addEventListener( + 'abort', + () => (fired = true) + ); + moduleScopeController.abort('done'); + strictEqual(fired, true); + strictEqual(moduleScopeController.signal.aborted, true); + strictEqual(moduleScopeController.signal.reason, 'done'); + }, +}; + +export const crossRequestNativeAbort = { + async test(ctrl, env, ctx) { + // Request A wraps a long native timer with a signal held in the remote end's global + // scope; request B then aborts it. The cancellation is delivered into A's context on its + // next turn, so A's wait rejects with the abort reason long before its 10s timeout. + const start = Date.now(); + const pending = env.RpcRemoteEnd.startAbortableWait(); + await scheduler.wait(100); + strictEqual(await env.RpcRemoteEnd.abortGlobalWait(), true); + const result = await pending; + match(result, /^aborted:/); + match(result, /cross-request/); + ok(Date.now() - start < 5000); + + // Aborting again is a no-op (and must not throw). + strictEqual(await env.RpcRemoteEnd.abortGlobalWait(), true); + }, +}; + +export const rpcSignalCancelsNativeWait = { + async test(ctrl, env, ctx) { + // The remote end wraps a long native wait with a signal it received over RPC; wrapping + // must arm the RPC abort subscription, so aborting our local controller cancels the + // remote wait long before its 10s timeout. + const start = Date.now(); + const ac = new AbortController(); + const pending = env.RpcRemoteEnd.waitOnReceivedSignal(ac.signal); + await scheduler.wait(100); + ac.abort(new Error('rpc-native-cancel')); + const result = await pending; + match(result, /^aborted:/); + match(result, /rpc-native-cancel/); + ok(Date.now() - start < 5000); + }, +}; + +export const crossRequestRegistrationChurn = { + async test(ctrl, env, ctx) { + // Many short signal-wrapped waits against one module-scope signal, each from its own + // request. Completed registrations are released with their requests and swept by later + // ones; none of this may disturb subsequent use of the signal. + for (let i = 0; i < 20; i++) { + strictEqual(await env.RpcRemoteEnd.churnWait(), 'ok'); + // Reclamation (rather than per-request accumulation) is observable in the + // registration count staying bounded: each completed wait's registration is cleared + // when its promise settles, and empty cells are swept by the next registration. (The + // RpcRemoteEnd entrypoint is this same worker, so the module-scope controller here is + // the very signal being wrapped remotely.) + ok( + moduleScopeChurnController.signal.getNativeRegistrationCountForTest() <= + 2 + ); + } + + // The signal is still fully functional after all that churn: aborting it works, and + // further attempts to use it reject with the abort reason. + await env.RpcRemoteEnd.abortChurnController(); + await rejects(env.RpcRemoteEnd.churnWait(), { message: /churn-done/ }); + strictEqual( + moduleScopeChurnController.signal.getNativeRegistrationCountForTest(), + 0 + ); + }, +}; + export const rpcCrossRequestSignal = { async test(ctrl, env, ctx) { - // Save an AbortController in the global scope - await env.RpcRemoteEnd.tryUsingGlobalAbortController(); - - // Try to use it again - await rejects( - async () => env.RpcRemoteEnd.tryUsingGlobalAbortController(), - { - name: 'Error', - message: - "Cannot perform I/O on behalf of a different request. I/O objects (such as streams, request/response bodies, and others) created in the context of one request handler cannot be accessed from a different request's handler. This is a limitation of Cloudflare Workers which allows us to improve overall performance. (I/O type: RefcountedCanceler)", - } + // Save an AbortController in the global scope of the remote end. Serializing its signal + // over RPC binds an RPC registration to that first request's context. + strictEqual( + await env.RpcRemoteEnd.tryUsingGlobalAbortController(), + 'created' ); + + // Abort it from a different request. The abort updates the signal's JS-visible state and + // fires its events; the first request's RPC registration died with that request and is + // dropped silently. + const res = await env.RpcRemoteEnd.tryUsingGlobalAbortController(); + strictEqual(res.aborted, true); + strictEqual(res.reason, 'boom?'); + }, +}; + +export const rpcCrossRequestListener = { + async test(ctrl, env, ctx) { + // A signal deserialized by one request is observed via addEventListener() from a second + // request while the first is still running. Only the receiving request can await the + // underlying RPC promise, so the second request's registration must route the arming of + // the subscription into the first request's context — otherwise the abort would update + // the pending-reason box but never fire the listener. + const ac = new AbortController(); + const held = env.RpcRemoteEnd.holdReceivedSignal(ac.signal); + strictEqual(await env.RpcRemoteEnd.listenOnHeldSignal(), false); + ac.abort(new Error('cross-request-listener')); + strictEqual(await held, 'aborted:cross-request-listener'); }, }; diff --git a/src/workerd/api/tests/abortsignal-test.wd-test b/src/workerd/api/tests/abortsignal-test.wd-test index 913e9102c40..626756f5ff2 100644 --- a/src/workerd/api/tests/abortsignal-test.wd-test +++ b/src/workerd/api/tests/abortsignal-test.wd-test @@ -7,7 +7,7 @@ const unitTests :Workerd.Config = ( modules = [ (name = "worker", esModule = embed "abortsignal-test.js") ], - compatibilityFlags = ["nodejs_compat", "enable_abortsignal_rpc", "experimental"], + compatibilityFlags = ["nodejs_compat", "enable_abortsignal_rpc", "experimental", "spec_compliant_dispatch_exceptions"], bindings = [ (name = "RpcRemoteEnd", service = (name = "abortsignal-test", entrypoint = "RpcRemoteEnd")), ] diff --git a/src/workerd/api/tests/cross-context-promise-test.js b/src/workerd/api/tests/cross-context-promise-test.js index ddc6f35257c..b6093d23453 100644 --- a/src/workerd/api/tests/cross-context-promise-test.js +++ b/src/workerd/api/tests/cross-context-promise-test.js @@ -6,6 +6,18 @@ import { AsyncLocalStorage } from 'async_hooks'; import { inspect } from 'util'; import { mock } from 'node:test'; +// Returns a probe function that is bound to the calling request's IoContext: invoking it +// succeeds in that context and throws "Cannot perform I/O on behalf of a different request" +// from any other. Several tests below use this to prove which IoContext a cross-request +// promise continuation runs in. An accepted WebSocket has the property we need because its +// native state is owned by the request that created it; its peer is never accepted, so sent +// probe messages just buffer. +function newIoContextProbe() { + const pair = new WebSocketPair(); + pair[0].accept(); + return () => pair[0].send('probe'); +} + export const crossContextResolveWorks = { async test(_, env) { // We're going to send two simultaneous requests to the same endpoint. @@ -194,19 +206,18 @@ async function resolveTest(req, env, ctx) { setupWaiter(ctx); const { promise, resolve } = Promise.withResolvers(); globalThis.request1 = { promise, resolve }; - const ab = AbortSignal.abort(); - strictEqual(ab.aborted, true); + const probe = newIoContextProbe(); + probe(); await als.run(123, async () => { await promise; strictEqual(als.getStore(), 123); }); // This part is the main test. It will not run until after the promise - // is resolved in the second request. - // We use an AbortSignal because it is bound to the IoContext and will - // throw an error if ab.aborted is checked from the wrong IoContext. - // If this line runes, it is proof that the promise continuation is - // running in the correct IoContext. - strictEqual(ab.aborted, true); + // is resolved in the second request. The probe is bound to this request's + // IoContext and throws if invoked from any other, so running without + // throwing here is proof that the promise continuation is running in the + // correct IoContext. + probe(); return new Response('ok'); } @@ -239,19 +250,19 @@ async function rejectTest(req, env, ctx) { setupWaiter(ctx); const { promise, reject } = Promise.withResolvers(); globalThis.request2 = { reject }; - const ab = AbortSignal.abort(); - strictEqual(ab.aborted, true); + const probe = newIoContextProbe(); + probe(); try { // The promise will be rejected from the other request. await promise; throw new Error('should not get here'); } catch (err) { // The reason provided by the other request should be carried - // through here. If the ab.aborted check throws, then the continuation - // is running in the wrong IoContext, which is the main thing we are - // testing for here. + // through here. If the probe throws, then the continuation is running + // in the wrong IoContext, which is the main thing we are testing for + // here. strictEqual(err, reason); - strictEqual(ab.aborted, true); + probe(); } return new Response('ok'); } @@ -277,10 +288,10 @@ async function crossRequestStream(req, env, ctx) { }); globalThis.stream = { controller }; const reader = readable.getReader(); - const ab = AbortSignal.abort(); - strictEqual(ab.aborted, true); + const probe = newIoContextProbe(); + probe(); const _read = await reader.read(); - strictEqual(ab.aborted, true); + probe(); return new Response('ok'); } @@ -300,31 +311,30 @@ async function customThenable(req, env, ctx) { setupWaiter(ctx); const { promise, resolve } = Promise.withResolvers(); globalThis.thenable = { resolve }; - const ab = AbortSignal.abort(); - strictEqual(ab.aborted, true); + const probe = newIoContextProbe(); + probe(); // We check to make sure the value provided by the custom thenable is // property passed through to the promise resolution. strictEqual(await promise, 1); // This part is the main test. It will not run until after the promise - // is resolved in the second request. - // We use an AbortSignal because it is bound to the IoContext and will - // throw an error if ab.aborted is checked from the wrong IoContext. - // If this line runes, it is proof that the promise continuation is - // running in the correct IoContext. - strictEqual(ab.aborted, true); + // is resolved in the second request. The probe is bound to this request's + // IoContext and throws if invoked from any other, so running without + // throwing here is proof that the promise continuation is running in the + // correct IoContext. + probe(); return new Response('ok'); } // This is our second request. Here, all we do is resolve the promise. - const ab = AbortSignal.abort(); - strictEqual(ab.aborted, true); + const probe = newIoContextProbe(); + probe(); const then = mock.fn((resolve) => { // The thenable should be invoked in the second request's IoContext. - // If it is not, then the ab.aborted check below will fail. - strictEqual(ab.aborted, true); + // If it is not, then this probe will throw. + probe(); resolve(1); }); @@ -346,8 +356,8 @@ async function unhandledRejection(req, env, ctx) { setupWaiter(ctx); const { reject } = Promise.withResolvers(); globalThis.unhandled = { reject }; - const ab = AbortSignal.abort(); - strictEqual(ab.aborted, true); + const probe = newIoContextProbe(); + probe(); const rejectPromise = Promise.withResolvers(); globalThis.addEventListener( @@ -355,9 +365,9 @@ async function unhandledRejection(req, env, ctx) { (event) => { // With deferred cross-context settlement, the rejection (and therefore // the unhandledrejection event) is dispatched in the owning IoContext, - // not the rejecting request's context. This means ab.aborted should - // work correctly here — we are in the right IoContext. - strictEqual(ab.aborted, true); + // not the rejecting request's context. The probe throwing here would + // mean we are in the wrong IoContext. + probe(); strictEqual(event.reason, reason); rejectPromise.resolve(); }, @@ -402,20 +412,20 @@ async function expiredContext(req, env, ctx) { return new Response('ok'); } -async function* gen(ab) { +async function* gen(probe) { let c = 0; for (;;) { await scheduler.wait(10); - strictEqual(ab.aborted, true); + probe(); yield c++; } } async function asyncIterator(req, env, ctx) { if (globalThis.asynciter === undefined) { - const ab = AbortSignal.abort(); - globalThis.asynciter = gen(ab); - globalThis.asyncIter2 = gen(ab); + const probe = newIoContextProbe(); + globalThis.asynciter = gen(probe); + globalThis.asyncIter2 = gen(probe); return new Response('ok'); } @@ -451,8 +461,8 @@ async function cyclicPromise(req, env, ctx) { setupWaiter(ctx); const { promise, resolve } = Promise.withResolvers(); globalThis.cyclic = { promise, resolve }; - const ab = AbortSignal.abort(); - strictEqual(ab.aborted, true); + const probe = newIoContextProbe(); + probe(); await promise; throw new Error('should never get here'); } @@ -493,8 +503,8 @@ async function resolveViaSubrequest(req, env, ctx) { // or ctx.waitUntil() to keep the request alive explicitly. const { promise, resolve } = Promise.withResolvers(); globalThis.resolveViaSubrequest = { resolve }; - const ab = AbortSignal.abort(); - strictEqual(ab.aborted, true); + const probe = newIoContextProbe(); + probe(); const res = await env.subrequest.fetch( 'http://example.org/resolve-via-subrequest-helper' @@ -502,7 +512,7 @@ async function resolveViaSubrequest(req, env, ctx) { strictEqual(res.status, 200); const result = await promise; - strictEqual(ab.aborted, true); + probe(); strictEqual(result, 'resolved-by-subrequest'); return new Response('ok'); } diff --git a/src/workerd/api/tests/events-test.js b/src/workerd/api/tests/events-test.js index 58f0a4eb705..c2d53a1bb96 100644 --- a/src/workerd/api/tests/events-test.js +++ b/src/workerd/api/tests/events-test.js @@ -375,8 +375,9 @@ export const globalIsEventTarget = { export const errorInHandler = { test() { - // TODO(bug): Erroring in one event handler should not prevent others from being - // run but we currently do not implement this correctly. + // A throwing event handler must not prevent the remaining handlers from running, nor + // propagate out of dispatchEvent(); the exception is reported to the global scope via + // the (cancelable) 'error' event. const event = new Event('foo'); const target = new EventTarget(); let dispatchCount = 0; @@ -388,11 +389,47 @@ export const errorInHandler = { dispatchCount++; }); - throws(() => target.dispatchEvent(event)); + let reported = null; + const errorHandler = (errEvent) => { + reported = errEvent.error; + // The report is handled; suppress the console fallback. + errEvent.preventDefault(); + }; + globalThis.addEventListener('error', errorHandler); + try { + strictEqual(target.dispatchEvent(event), true); + } finally { + globalThis.removeEventListener('error', errorHandler); + } - // The dispatchCount here should be 2, but with the current bug, it's only 1 - // strictEqual(dispatchCount, 2); - strictEqual(dispatchCount, 1); + strictEqual(dispatchCount, 2); + strictEqual(reported?.message, 'boom'); + }, +}; + +export const listenersAddedDuringDispatch = { + test() { + // Listeners added while an event is being dispatched do not run for the in-flight + // event, but do run for subsequent dispatches — including when many are added at once + // (which historically stressed handler storage mutation during iteration). + const target = new EventTarget(); + let outer = 0; + let added = 0; + target.addEventListener('foo', () => { + outer++; + for (let i = 0; i < 16; i++) { + target.addEventListener('foo', () => added++); + } + }); + target.addEventListener('foo', () => outer++); + + target.dispatchEvent(new Event('foo')); + strictEqual(outer, 2); + strictEqual(added, 0); + + target.dispatchEvent(new Event('foo')); + strictEqual(outer, 4); + strictEqual(added, 16); }, }; @@ -463,3 +500,345 @@ export const handlerThis = { strictEqual(handlerObject.handleEvent.mock.callCount(), 1); }, }; + +export const isTrustedDefaults = { + async test() { + // User-constructed events are never trusted... + const userEvents = [ + new Event('foo'), + new CustomEvent('foo'), + new MessageEvent('foo', { data: 'bar' }), + new ErrorEvent('foo'), + new CloseEvent('foo'), + ]; + for (const event of userEvents) { + strictEqual(event.isTrusted, false); + // ...including when observed by a listener during dispatch. + const target = new EventTarget(); + let trusted; + target.addEventListener('foo', (e) => { + trusted = e.isTrusted; + }); + target.dispatchEvent(event); + strictEqual(trusted, false); + } + + // Events constructed and dispatched by the runtime are trusted. + { + const ac = new AbortController(); + let trusted; + ac.signal.addEventListener('abort', (e) => { + trusted = e.isTrusted; + }); + ac.abort(); + strictEqual(trusted, true); + } + + { + const { promise, resolve } = Promise.withResolvers(); + const handler = (e) => resolve(e.isTrusted); + addEventListener('unhandledrejection', handler); + Promise.reject(new Error('boom')); + strictEqual(await promise, true); + removeEventListener('unhandledrejection', handler); + } + }, +}; + +// Under the REPORT dispatch policy, a listener exception is reported to the global scope's +// 'error' event synchronously, between the throwing listener and the next one. +export const reportedListenerErrorInterleaving = { + test() { + const order = []; + const boom = new Error('boom'); + const globalHandler = (event) => { + order.push('global-error'); + strictEqual(event.error, boom); + }; + addEventListener('error', globalHandler); + try { + const target = new EventTarget(); + target.addEventListener('foo', () => { + order.push('l1'); + throw boom; + }); + target.addEventListener('foo', () => order.push('l2')); + // dispatchEvent() itself must not throw. + target.dispatchEvent(new Event('foo')); + deepStrictEqual(order, ['l1', 'global-error', 'l2']); + } finally { + removeEventListener('error', globalHandler); + } + }, +}; + +// A throwing global 'error' listener must not break the REPORT no-throw contract: the +// nested report is routed to the console (HTML's "in error reporting mode" guard) instead +// of propagating or recursing. +export const throwingGlobalErrorListener = { + test() { + const order = []; + const globalHandler = () => { + order.push('global-error'); + throw new Error('error handler boom'); + }; + addEventListener('error', globalHandler); + try { + // Via a REPORT dispatch on an EventTarget. + const target = new EventTarget(); + target.addEventListener('foo', () => { + order.push('l1'); + throw new Error('boom'); + }); + target.addEventListener('foo', () => order.push('l2')); + target.dispatchEvent(new Event('foo')); + deepStrictEqual(order, ['l1', 'global-error', 'l2']); + + // Via AbortController.abort(), which the spec forbids from throwing. + order.length = 0; + const ac = new AbortController(); + ac.signal.addEventListener('abort', () => { + order.push('abort1'); + throw new Error('abort boom'); + }); + ac.signal.addEventListener('abort', () => order.push('abort2')); + ac.abort(); + deepStrictEqual(order, ['abort1', 'global-error', 'abort2']); + + // Via reportError() directly. + reportError(new Error('reported boom')); + } finally { + removeEventListener('error', globalHandler); + } + }, +}; + +// The report-an-exception console fallback reads `error.stack`, which can run arbitrary +// user code (a getter or proxy trap). A throwing stack getter must not escape any of the +// no-throw report paths: reportError() itself, a REPORT dispatch, or the nested +// (in-error-reporting-mode) report. +export const throwingStackGetter = { + test() { + const makeEvil = (msg) => ({ + get stack() { + throw new Error(`evil stack: ${msg}`); + }, + }); + + // Via reportError() directly: must not throw. + reportError(makeEvil('direct')); + + // Via a REPORT dispatch: the listener's thrown value has a throwing stack getter; + // dispatchEvent() must not throw and the remaining listeners still run. + const order = []; + const target = new EventTarget(); + target.addEventListener('foo', () => { + order.push('l1'); + throw makeEvil('listener'); + }); + target.addEventListener('foo', () => order.push('l2')); + target.dispatchEvent(new Event('foo')); + deepStrictEqual(order, ['l1', 'l2']); + + // Via abort(), which the spec forbids from throwing. + const ac = new AbortController(); + ac.signal.addEventListener('abort', () => { + order.push('abort1'); + throw makeEvil('abort'); + }); + ac.signal.addEventListener('abort', () => order.push('abort2')); + ac.abort(); + deepStrictEqual(order, ['l1', 'l2', 'abort1', 'abort2']); + + // Via the nested report: a global 'error' listener throws a value whose stack getter + // throws. The nested report goes to the console and must neither propagate nor stop + // the original dispatch. + order.length = 0; + const globalHandler = () => { + order.push('global-error'); + throw makeEvil('nested'); + }; + addEventListener('error', globalHandler); + try { + target.dispatchEvent(new Event('foo')); + deepStrictEqual(order, ['l1', 'global-error', 'l2']); + } finally { + removeEventListener('error', globalHandler); + } + }, +}; + +// User code running during the mid-dispatch report can mutate the original listener list; +// removals are honored for listeners that have not run yet. +export const midReportListenerRemoval = { + test() { + const order = []; + const target = new EventTarget(); + const l2 = () => order.push('l2'); + const globalHandler = () => { + order.push('global-error'); + target.removeEventListener('foo', l2); + }; + addEventListener('error', globalHandler); + try { + target.addEventListener('foo', () => { + order.push('l1'); + throw new Error('boom'); + }); + target.addEventListener('foo', l2); + target.dispatchEvent(new Event('foo')); + deepStrictEqual(order, ['l1', 'global-error']); + } finally { + removeEventListener('error', globalHandler); + } + }, +}; + +// A throwing WebSocket 'message' listener has its exception reported and the remaining +// listeners still run, but the WebSocket is still errored out afterwards (fail-fast). +export const webSocketThrowingMessageListener = { + async test() { + const order = []; + const boom = new Error('ws boom'); + const globalHandler = () => order.push('global-error'); + addEventListener('error', globalHandler); + try { + const { 0: client, 1: server } = new WebSocketPair(); + client.accept(); + server.accept(); + + const errorPromise = new Promise((resolve) => { + client.addEventListener('error', (event) => resolve(event.error)); + }); + const l2Promise = new Promise((resolve) => { + client.addEventListener('message', () => { + order.push('l1'); + throw boom; + }); + client.addEventListener('message', () => { + // The fail-fast teardown happens strictly after the dispatch completes: this + // listener still observes a live, usable WebSocket even though the previous + // listener threw. + order.push(`l2:readyState=${client.readyState}`); + client.send('still-works'); + resolve(); + }); + }); + + const serverReceived = new Promise((resolve) => { + server.addEventListener('message', (event) => resolve(event.data)); + }); + + server.send('hello'); + await l2Promise; + deepStrictEqual(order, [ + 'l1', + 'global-error', + `l2:readyState=${WebSocket.READY_STATE_OPEN}`, + ]); + // The send() from the second listener made it out before the teardown. + strictEqual(await serverReceived, 'still-works'); + // The fail-fast reaction errors the WebSocket with the listener's exception. The + // exception crosses the JS/KJ boundary in the read loop and is reconstructed, so + // only the message survives. + ok(String(await errorPromise).includes('ws boom')); + } finally { + removeEventListener('error', globalHandler); + } + }, +}; + +// The standard MessageEventInit members are all supported (and optional) for +// user-constructed events, with spec defaults. +export const messageEventSpecInit = { + test() { + const defaults = new MessageEvent('message'); + strictEqual(defaults.data, null); + strictEqual(defaults.origin, ''); + strictEqual(defaults.lastEventId, ''); + strictEqual(defaults.source, null); + deepStrictEqual(defaults.ports, []); + strictEqual(defaults.bubbles, false); + strictEqual(defaults.cancelable, false); + strictEqual(defaults.composed, false); + + const data = { hello: 'world' }; + const event = new MessageEvent('message', { + data, + origin: 'https://example.org', + lastEventId: '42', + bubbles: true, + cancelable: true, + composed: true, + }); + strictEqual(event.data, data); + strictEqual(event.origin, 'https://example.org'); + strictEqual(event.lastEventId, '42'); + strictEqual(event.bubbles, true); + strictEqual(event.cancelable, true); + strictEqual(event.composed, true); + event.preventDefault(); + strictEqual(event.defaultPrevented, true); + }, +}; + +// CloseEventInit supports the common EventInit members. +export const closeEventSpecInit = { + test() { + const defaults = new CloseEvent('close'); + strictEqual(defaults.code, 0); + strictEqual(defaults.reason, ''); + strictEqual(defaults.wasClean, false); + strictEqual(defaults.bubbles, false); + strictEqual(defaults.cancelable, false); + strictEqual(defaults.composed, false); + + const event = new CloseEvent('close', { + code: 1000, + reason: 'done', + wasClean: true, + bubbles: true, + cancelable: true, + composed: true, + }); + strictEqual(event.code, 1000); + strictEqual(event.reason, 'done'); + strictEqual(event.wasClean, true); + strictEqual(event.bubbles, true); + strictEqual(event.cancelable, true); + strictEqual(event.composed, true); + }, +}; + +// ErrorEventInit supports the common EventInit members. +export const errorEventSpecInit = { + test() { + const defaults = new ErrorEvent('error'); + strictEqual(defaults.message, ''); + strictEqual(defaults.bubbles, false); + strictEqual(defaults.cancelable, false); + strictEqual(defaults.composed, false); + + const err = new Error('boom'); + const event = new ErrorEvent('error', { + message: 'boom', + filename: 'test.js', + lineno: 1, + colno: 2, + error: err, + bubbles: true, + cancelable: true, + composed: true, + }); + strictEqual(event.message, 'boom'); + strictEqual(event.filename, 'test.js'); + strictEqual(event.lineno, 1); + strictEqual(event.colno, 2); + strictEqual(event.error, err); + strictEqual(event.bubbles, true); + strictEqual(event.cancelable, true); + strictEqual(event.composed, true); + event.preventDefault(); + strictEqual(event.defaultPrevented, true); + }, +}; diff --git a/src/workerd/api/tests/events-test.wd-test b/src/workerd/api/tests/events-test.wd-test index b87cd682f09..f06a7d9e871 100644 --- a/src/workerd/api/tests/events-test.wd-test +++ b/src/workerd/api/tests/events-test.wd-test @@ -7,7 +7,7 @@ const unitTests :Workerd.Config = ( modules = [ (name = "worker", esModule = embed "events-test.js") ], - compatibilityFlags = ["nodejs_compat", "set_event_target_this", "workers_api_getters_setters_on_prototype", "dont_substitute_null_on_type_error"] + compatibilityFlags = ["nodejs_compat", "set_event_target_this", "workers_api_getters_setters_on_prototype", "dont_substitute_null_on_type_error", "spec_compliant_dispatch_exceptions"] ) ), ], diff --git a/src/workerd/api/tests/eventsource-test.js b/src/workerd/api/tests/eventsource-test.js index 7e3de8b4336..820d8dacb05 100644 --- a/src/workerd/api/tests/eventsource-test.js +++ b/src/workerd/api/tests/eventsource-test.js @@ -1,7 +1,7 @@ // Copyright (c) 2017-2024 Cloudflare, Inc. // Licensed under the Apache 2.0 license found in the LICENSE file or at: // https://opensource.org/licenses/Apache-2.0 -import { strictEqual, ok, throws } from 'node:assert'; +import { deepStrictEqual, strictEqual, ok, throws } from 'node:assert'; export const acceptEventStreamTest = { async test(ctrl, env) { @@ -611,3 +611,88 @@ export default { return await handler(request); }, }; + +// The onmessage handler occupies a normal position in the listener list based on when it +// was first assigned, per HTML's event handler semantics. The same machinery backs onopen +// and onerror. +export const onmessagePositionalOrdering = { + async test() { + const enc = new TextEncoder(); + const rs = new ReadableStream({ + pull(c) { + c.enqueue(enc.encode('data: hello\n\n')); + c.close(); + }, + }); + const order = []; + const { promise, resolve } = Promise.withResolvers(); + const eventsource = EventSource.from(rs); + + eventsource.addEventListener('message', () => order.push('a')); + const b1 = () => order.push('b1'); + eventsource.onmessage = b1; + strictEqual(eventsource.onmessage, b1); + eventsource.addEventListener('message', () => order.push('c')); + + // Reassignment keeps the original position; clearing and reassigning would take a + // fresh position at the end. + eventsource.onmessage = () => order.push('b2'); + + eventsource.addEventListener('message', () => { + eventsource.close(); + resolve(); + }); + await promise; + deepStrictEqual(order, ['a', 'b2', 'c']); + }, +}; + +// A throwing 'message' listener has its exception reported (and the remaining listeners +// for that event still run), then the EventSource is errored out (fail-fast): the 'error' +// event fires, the stream closes, and the remaining messages in the batch are dropped. +export const throwingMessageListener = { + async test() { + const order = []; + const boom = new Error('es boom'); + const globalHandler = () => order.push('global-error'); + addEventListener('error', globalHandler); + try { + const enc = new TextEncoder(); + const rs = new ReadableStream({ + pull(c) { + // Both messages arrive in a single batch. + c.enqueue(enc.encode('data: first\n\ndata: second\n\n')); + c.close(); + }, + }); + const eventsource = EventSource.from(rs); + const errorPromise = new Promise((resolve) => { + eventsource.addEventListener('error', (event) => { + order.push('es-error'); + resolve(event.error); + }); + }); + eventsource.addEventListener('message', (event) => { + order.push(`l1:${event.data}`); + throw boom; + }); + eventsource.addEventListener('message', (event) => { + // The fail-fast error happens strictly after the dispatch completes: this listener + // still observes an open EventSource even though the previous listener threw. + order.push(`l2:${event.data}:readyState=${eventsource.readyState}`); + }); + + strictEqual(await errorPromise, boom); + strictEqual(eventsource.readyState, EventSource.CLOSED); + // 'second' was dropped along with the rest of the batch. + deepStrictEqual(order, [ + 'l1:first', + 'global-error', + `l2:first:readyState=${EventSource.OPEN}`, + 'es-error', + ]); + } finally { + removeEventListener('error', globalHandler); + } + }, +}; diff --git a/src/workerd/api/tests/eventsource-test.wd-test b/src/workerd/api/tests/eventsource-test.wd-test index 248eb99dcf8..62dbf4d1d80 100644 --- a/src/workerd/api/tests/eventsource-test.wd-test +++ b/src/workerd/api/tests/eventsource-test.wd-test @@ -7,7 +7,7 @@ const unitTests :Workerd.Config = ( modules = [ (name = "worker", esModule = embed "eventsource-test.js") ], - compatibilityFlags = ["nodejs_compat", "experimental", "streams_enable_constructors"], + compatibilityFlags = ["nodejs_compat", "experimental", "streams_enable_constructors", "spec_compliant_dispatch_exceptions"], bindings = [ (name = "subrequest", service = "eventsource-test") ] diff --git a/src/workerd/api/tests/legacy-dispatch-exceptions-test.js b/src/workerd/api/tests/legacy-dispatch-exceptions-test.js new file mode 100644 index 00000000000..807b739dbb7 --- /dev/null +++ b/src/workerd/api/tests/legacy-dispatch-exceptions-test.js @@ -0,0 +1,89 @@ +// Copyright (c) 2026 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 +import { deepStrictEqual, strictEqual, throws } from 'node:assert'; +import { mock } from 'node:test'; + +// With spec_compliant_dispatch_exceptions disabled (pinned off via the disable flag so +// every test variant runs this path), event dispatch keeps the legacy PROPAGATE +// semantics: the first throwing listener ends the dispatch and the exception propagates +// to the dispatching code. + +export const dispatchEventPropagates = { + test() { + const target = new EventTarget(); + const boom = new Error('boom'); + const l2 = mock.fn(); + target.addEventListener('foo', () => { + throw boom; + }); + target.addEventListener('foo', l2); + // The exception propagates out of dispatchEvent() and the remaining listeners for + // the event are skipped. + throws(() => target.dispatchEvent(new Event('foo')), boom); + strictEqual(l2.mock.callCount(), 0); + }, +}; + +export const abortPropagates = { + test() { + const ac = new AbortController(); + const boom = new Error('abort boom'); + const l2 = mock.fn(); + ac.signal.addEventListener('abort', () => { + throw boom; + }); + ac.signal.addEventListener('abort', l2); + throws(() => ac.abort(), boom); + strictEqual(l2.mock.callCount(), 0); + }, +}; + +// A throwing 'message' listener's exception is swallowed and re-dispatched as a second +// 'message' event carrying the exception as its data. No 'messageerror' event fires, and +// the port keeps delivering afterwards. +export const messagePortLegacyRedispatch = { + async test() { + const { port1, port2 } = new MessageChannel(); + const boom = new Error('port boom'); + const seen = []; + const done = Promise.withResolvers(); + const messageerror = mock.fn(); + port2.addEventListener('messageerror', messageerror); + port2.addEventListener('message', (event) => { + seen.push(event.data); + if (event.data === 'bad') throw boom; + if (event.data === 'after') done.resolve(); + }); + + port1.postMessage('bad'); + port1.postMessage('after'); + await done.promise; + deepStrictEqual(seen, ['bad', boom, 'after']); + strictEqual(messageerror.mock.callCount(), 0); + }, +}; + +// A throwing 'message' listener errors out the EventSource: the 'error' event fires with +// the listener's exception and the stream closes. +export const eventSourceLegacyError = { + async test() { + const enc = new TextEncoder(); + const rs = new ReadableStream({ + pull(c) { + c.enqueue(enc.encode('data: first\n\n')); + c.close(); + }, + }); + const boom = new Error('es boom'); + const eventsource = EventSource.from(rs); + const errorPromise = new Promise((resolve) => { + eventsource.addEventListener('error', (event) => resolve(event.error)); + }); + eventsource.addEventListener('message', () => { + throw boom; + }); + strictEqual(await errorPromise, boom); + strictEqual(eventsource.readyState, EventSource.CLOSED); + }, +}; diff --git a/src/workerd/api/tests/legacy-dispatch-exceptions-test.wd-test b/src/workerd/api/tests/legacy-dispatch-exceptions-test.wd-test new file mode 100644 index 00000000000..8bec832016b --- /dev/null +++ b/src/workerd/api/tests/legacy-dispatch-exceptions-test.wd-test @@ -0,0 +1,16 @@ +using Workerd = import "/workerd/workerd.capnp"; + +const unitTests :Workerd.Config = ( + services = [ + ( name = "legacy-dispatch-exceptions-test", + worker = ( + modules = [ + (name = "worker", esModule = embed "legacy-dispatch-exceptions-test.js") + ], + # The disable flag is pinned so that every test variant — including + # @all-compat-flags — exercises the legacy PROPAGATE dispatch path. + compatibilityFlags = ["nodejs_compat", "experimental", "streams_enable_constructors", "expose_global_message_channel", "no_spec_compliant_dispatch_exceptions"], + ) + ), + ], +); diff --git a/src/workerd/api/tests/messageport-test.js b/src/workerd/api/tests/messageport-test.js index 64f3d049b8b..b599e631967 100644 --- a/src/workerd/api/tests/messageport-test.js +++ b/src/workerd/api/tests/messageport-test.js @@ -1,7 +1,7 @@ // Copyright (c) 2025 Cloudflare, Inc. // Licensed under the Apache 2.0 license found in the LICENSE file or at: // https://opensource.org/licenses/Apache-2.0 -import { ok, strictEqual, throws } from 'node:assert'; +import { deepStrictEqual, ok, strictEqual, throws } from 'node:assert'; import { mock } from 'node:test'; @@ -150,3 +150,218 @@ export const postMessageRpcTarget = { // and we might not ever. Need to investigate this further but it's not blocking us // right now. // * https://github.com/web-platform-tests/wpt/blob/master/webmessaging/message-channels/close-event/garbage-collected.tentative.any.js + +// The onmessage handler occupies a normal position in the listener list based on when it +// was first assigned, per HTML's event handler semantics. +export const onmessagePositionalOrdering = { + async test() { + const { port1, port2 } = new MessageChannel(); + const order = []; + const { promise, resolve } = Promise.withResolvers(); + + port2.addEventListener('message', () => order.push('a')); + const b1 = () => order.push('b1'); + port2.onmessage = b1; + strictEqual(port2.onmessage, b1); + port2.addEventListener('message', () => order.push('c')); + + // Reassignment keeps the original position. + port2.onmessage = () => order.push('b2'); + + port2.addEventListener('message', () => resolve()); + port1.postMessage('hello'); + await promise; + deepStrictEqual(order, ['a', 'b2', 'c']); + }, +}; + +// Clearing onmessage and assigning it again takes a fresh position at the end of the +// listener list. +export const onmessageClearedTakesFreshPosition = { + async test() { + const { port1, port2 } = new MessageChannel(); + const order = []; + const { promise, resolve } = Promise.withResolvers(); + + port2.onmessage = () => order.push('handler1'); + port2.addEventListener('message', () => order.push('listener')); + + port2.onmessage = null; + strictEqual(port2.onmessage, null); + port2.onmessage = () => order.push('handler2'); + + port2.addEventListener('message', () => resolve()); + port1.postMessage('hello'); + await promise; + deepStrictEqual(order, ['listener', 'handler2']); + }, +}; + +// Assigning a non-callable object to onmessage retains it as the attribute value and +// enables the port's message queue, but the object is never invoked: messages delivered +// while it is assigned are consumed and dropped. +export const onmessageNonCallableStartsPort = { + async test() { + const { port1, port2 } = new MessageChannel(); + const obj = {}; + port2.onmessage = obj; + strictEqual(port2.onmessage, obj); + port1.postMessage('lost'); + await scheduler.wait(10); + + const { promise, resolve } = Promise.withResolvers(); + port2.onmessage = (event) => resolve(event.data); + port1.postMessage('kept'); + strictEqual(await promise, 'kept'); + }, +}; + +// Adding a 'message' listener via addEventListener starts the port, the same as assigning +// onmessage (Node.js behavior; per spec only the onmessage attribute enables the queue). +export const addEventListenerStartsPort = { + async test() { + const { port1, port2 } = new MessageChannel(); + const { promise, resolve } = Promise.withResolvers(); + port2.addEventListener('message', (event) => resolve(event.data)); + port1.postMessage('hello'); + strictEqual(await promise, 'hello'); + }, +}; + +// Removing the last 'message' listener returns the port to the pending state: messages +// queue (rather than being dropped) until another listener is attached. +export const removingLastListenerRequeues = { + async test() { + const { port1, port2 } = new MessageChannel(); + const first = Promise.withResolvers(); + const handler = (event) => first.resolve(event.data); + port2.addEventListener('message', handler); + port1.postMessage('one'); + strictEqual(await first.promise, 'one'); + + port2.removeEventListener('message', handler); + port1.postMessage('two'); + await scheduler.wait(10); + + const second = Promise.withResolvers(); + port2.addEventListener('message', (event) => second.resolve(event.data)); + strictEqual(await second.promise, 'two'); + }, +}; + +// A once-listener starts the port; its removal after the first message returns the port +// to pending, so later messages queue until a new listener arrives. +export const onceListenerReturnsPortToPending = { + async test() { + const { port1, port2 } = new MessageChannel(); + const first = Promise.withResolvers(); + port2.addEventListener('message', (event) => first.resolve(event.data), { + once: true, + }); + port1.postMessage('one'); + strictEqual(await first.promise, 'one'); + + port1.postMessage('two'); + await scheduler.wait(10); + + const second = Promise.withResolvers(); + port2.addEventListener('message', (event) => second.resolve(event.data)); + strictEqual(await second.promise, 'two'); + }, +}; + +// A closed port is terminal: attaching listeners or manipulating onmessage afterwards +// never restarts it, and no messages are delivered. +export const closedPortIsTerminal = { + async test() { + const { port1, port2 } = new MessageChannel(); + port1.postMessage('queued'); + port2.close(); + + const handler = mock.fn(); + port2.onmessage = null; + port2.onmessage = handler; + port2.addEventListener('message', handler); + port1.postMessage('late'); + await scheduler.wait(10); + strictEqual(handler.mock.callCount(), 0); + }, +}; + +// A throwing 'message' listener has its exception reported (and the remaining listeners +// still run), and the port additionally dispatches a 'messageerror' event carrying the +// exception. The port itself keeps working. +export const throwingMessageListener = { + async test() { + const order = []; + const boom = new Error('port boom'); + const { port1, port2 } = new MessageChannel(); + const globalHandler = () => { + order.push('global-error'); + // Injecting a message mid-report cannot jump the queue: delivery is always deferred + // to a later microtask, so it arrives after the current event's remaining listeners, + // after the synthesized messageerror, and after any messages queued before it. + port1.postMessage('injected'); + }; + globalThis.addEventListener('error', globalHandler); + try { + const done = Promise.withResolvers(); + port2.addEventListener('message', (event) => { + order.push(`l1:${event.data}`); + if (event.data === 'bad') throw boom; + if (event.data === 'injected') done.resolve(); + }); + port2.addEventListener('message', (event) => + order.push(`l2:${event.data}`) + ); + port2.addEventListener('messageerror', (event) => { + order.push('messageerror'); + strictEqual(event.data, boom); + }); + + port1.postMessage('bad'); + port1.postMessage('after'); + await done.promise; + deepStrictEqual(order, [ + 'l1:bad', + 'global-error', + 'l2:bad', + 'messageerror', + 'l1:after', + 'l2:after', + 'l1:injected', + 'l2:injected', + ]); + } finally { + globalThis.removeEventListener('error', globalHandler); + } + }, +}; + +// User-constructed MessageEvents reflect the source and ports passed in their init. +// (The runtime itself never attaches either: ports are not transferable here.) +export const messageEventSourceAndPorts = { + test() { + const { port1, port2 } = new MessageChannel(); + const event = new MessageEvent('message', { + data: 'x', + source: port1, + ports: [port1, port2], + }); + strictEqual(event.source, port1); + const ports = event.ports; + strictEqual(ports.length, 2); + strictEqual(ports[0], port1); + strictEqual(ports[1], port2); + + // Runtime-delivered message events carry the entangled port as source and no ports. + const { promise, resolve } = Promise.withResolvers(); + port2.onmessage = (e) => resolve(e); + port1.postMessage('hi'); + return promise.then((e) => { + strictEqual(e.source, port2); + strictEqual(e.ports.length, 0); + strictEqual(e.origin, null); + }); + }, +}; diff --git a/src/workerd/api/tests/messageport-test.wd-test b/src/workerd/api/tests/messageport-test.wd-test index 6cec6568e81..aa5411b913a 100644 --- a/src/workerd/api/tests/messageport-test.wd-test +++ b/src/workerd/api/tests/messageport-test.wd-test @@ -7,7 +7,7 @@ const unitTests :Workerd.Config = ( modules = [ (name = "worker", esModule = embed "messageport-test.js") ], - compatibilityFlags = ["nodejs_compat_v2", "expose_global_message_channel"], + compatibilityFlags = ["nodejs_compat_v2", "expose_global_message_channel", "spec_compliant_dispatch_exceptions"], ) ), ], diff --git a/src/workerd/api/tests/reporterror-test.js b/src/workerd/api/tests/reporterror-test.js index c35a409f4e6..64d36b13f93 100644 --- a/src/workerd/api/tests/reporterror-test.js +++ b/src/workerd/api/tests/reporterror-test.js @@ -14,6 +14,7 @@ const expectedFilename = Cloudflare.compatibilityFlags.new_module_registry : 'worker'; const handler = mock.fn((event) => { + strictEqual(event.isTrusted, true); if (event.error instanceof Error) { strictEqual(event.message, 'Uncaught Error: boom'); strictEqual(event.colno, 13); @@ -23,7 +24,7 @@ const handler = mock.fn((event) => { } else { strictEqual(event.message, 'Uncaught boom'); strictEqual(event.colno, 0); - strictEqual(event.lineno, 35); + strictEqual(event.lineno, 36); strictEqual(event.filename, expectedFilename); strictEqual(event.error, 'boom'); } diff --git a/src/workerd/api/tests/streams-test.js b/src/workerd/api/tests/streams-test.js index 405761a9978..9915e724694 100644 --- a/src/workerd/api/tests/streams-test.js +++ b/src/workerd/api/tests/streams-test.js @@ -5,6 +5,17 @@ import { strictEqual, ok, deepStrictEqual, rejects, throws } from 'node:assert'; const enc = new TextEncoder(); +// A standard WritableStream allocates an AbortSignal for its controller, which must not +// require an active IoContext: constructing one at module scope works. +const moduleScopeWritable = new WritableStream(); + +export const globalScopeWritableStream = { + test() { + ok(moduleScopeWritable instanceof WritableStream); + strictEqual(moduleScopeWritable.locked, false); + }, +}; + export const rs = { async test(ctrl, env) { const resp = await env.subrequest.fetch('http://example.org', { diff --git a/src/workerd/api/web-socket.c++ b/src/workerd/api/web-socket.c++ index 098fce96948..d2976b91c13 100644 --- a/src/workerd/api/web-socket.c++ +++ b/src/workerd/api/web-socket.c++ @@ -29,6 +29,35 @@ namespace workerd::api { namespace { +// Dispatches a UA-fired WebSocket event with spec semantics (listener exceptions are +// reported and the remaining listeners still run), then rethrows the first listener +// exception, if any, so the caller's fail-fast error path engages and errors out the +// WebSocket. When the compat flag is not set, falls back to PROPAGATE (the old behavior, +// where the first throwing listener ends the dispatch and the exception propagates directly). +void dispatchWithFailFast(jsg::Lock& js, WebSocket& shell, jsg::Ref event) { + auto policy = + EventTarget::effectiveExceptionPolicy(js, EventTarget::DispatchExceptionPolicy::REPORT); + auto result = shell.dispatchEventImpl(js, kj::mv(event), policy); + KJ_IF_SOME(exception, result.firstException) { + js.throwException(exception.getHandle(js)); + } +} + +// Dispatches a UA-fired WebSocket event report-only: listener exceptions are reported and +// the dispatch continues, with no further reaction. Used for the 'close' and 'error' +// events, which fire when the WebSocket is already closed or failed — erroring it out +// again is useless, and rethrowing would only re-surface an already-reported exception +// into terminal plumbing (and skip the cleanup that follows the dispatch). When the compat +// flag is not set, falls back to PROPAGATE (the old behavior). +void dispatchReportOnly(jsg::Lock& js, WebSocket& shell, jsg::Ref event) { + shell.dispatchEventImpl(js, kj::mv(event), + EventTarget::effectiveExceptionPolicy(js, EventTarget::DispatchExceptionPolicy::REPORT)); +} + +} // namespace + +namespace { + // Emits a perf-counter mark for a WebSocket event from the current in-scope point (the JS send() // call or the readLoop message dispatch). No-op when not in an IoContext. The IsolateLimitEnforcer // implementation captures the timestamp, so this side stays time-agnostic and works on every @@ -361,8 +390,8 @@ void LegacyWebSocketAdapter::initConnection(jsg::Lock& js, kj::Promise(1006, kj::str("Failed to establish websocket connection"), false)); + dispatchReportOnly(js, shell, + js.alloc(1006, kj::str("Failed to establish websocket connection"), false)); }); // Note that in this attach we pass a strong reference to the WebSocket. The reference will be // dropped when either the connection promise completes or the IoContext is torn down, @@ -796,7 +825,7 @@ void LegacyWebSocketAdapter::startReadLoop( KJ_IF_SOME(e, maybeError) { if (!native.closedIncoming && e.getType() == kj::Exception::Type::DISCONNECTED) { // Report premature disconnect or cancel as a close event. - shell.dispatchEventImpl(js, + dispatchReportOnly(js, shell, js.alloc( 1006, kj::str("WebSocket disconnected without sending Close frame."), false)); native.closedIncoming = true; @@ -1058,7 +1087,7 @@ kj::Maybe LegacyWebSocketAdapter::getAutoResponseTimestamp() { void LegacyWebSocketAdapter::dispatchOpen(jsg::Lock& js) { constexpr kj::StringPtr kOpenEvent = "open"_kj; - shell.dispatchEventImpl(js, js.alloc(kOpenEvent)); + dispatchWithFailFast(js, shell, js.alloc(kOpenEvent, Event::Init{}, Trusted::YES)); } void LegacyWebSocketAdapter::ensurePumping(jsg::Lock& js) { @@ -1355,18 +1384,22 @@ kj::Promise> LegacyWebSocketAdapter::readLoop( markWebSocketPerfEvent("ws_received"_kjc); KJ_SWITCH_ONEOF(message) { KJ_CASE_ONEOF(text, kj::String) { - shell.dispatchEventImpl(js, js.alloc(js, js.str(text))); + dispatchWithFailFast(js, shell, + js.alloc( + js, js.str(text), kj::String(), kj::none, kj::none, Trusted::YES)); } KJ_CASE_ONEOF(data, kj::Array) { if (binaryType_ == BinaryType::BLOB) { // Per the WHATWG spec, deliver binary messages as Blob when binaryType is "blob". auto ab = jsg::JsArrayBuffer::create(js, data); auto blob = js.alloc(js, jsg::JsBufferSource(ab), kj::str()); - shell.dispatchEventImpl( - js, js.alloc(js, kj::str("message"), kj::mv(blob))); + dispatchWithFailFast(js, shell, + js.alloc(js, kj::str("message"), kj::mv(blob), kj::String(), + kj::none, kj::none, Trusted::YES)); } else { jsg::JsValue ab = jsg::JsArrayBuffer::create(js, data); - shell.dispatchEventImpl(js, js.alloc(js, ab)); + dispatchWithFailFast(js, shell, + js.alloc(js, ab, kj::String(), kj::none, kj::none, Trusted::YES)); } } KJ_CASE_ONEOF(close, kj::WebSocket::Close) { @@ -1388,8 +1421,8 @@ kj::Promise> LegacyWebSocketAdapter::readLoop( closedOutgoingForHib = true; ensurePumping(js); } - shell.dispatchEventImpl( - js, js.alloc(close.code, kj::mv(close.reason), true)); + dispatchReportOnly( + js, shell, js.alloc(close.code, kj::mv(close.reason), true)); // Native WebSocket no longer needed; release. tryReleaseNative(js); return false; @@ -1436,9 +1469,13 @@ void LegacyWebSocketAdapter::reportError(jsg::Lock& js, jsg::JsRef auto msg = kj::str(v8::Exception::CreateMessage(js.v8Isolate, err.getHandle(js))->Get()); error = err.addRef(js); + // Report-only dispatch: the WebSocket is already errored out at this point, so a + // throwing 'error' listener has its exception reported but triggers no further + // fail-fast reaction. shell.dispatchEventImpl(js, js.alloc( - ErrorEvent::ErrorEventInit{.message = kj::mv(msg), .error = kj::mv(err)})); + ErrorEvent::ErrorEventInit{.message = kj::mv(msg), .error = kj::mv(err)}), + EventTarget::effectiveExceptionPolicy(js, EventTarget::DispatchExceptionPolicy::REPORT)); // After an error we don't allow further send()s. If the receive loop has also ended then we // can destroy the connection. Note that we don't set closedOutgoing = true because that flag diff --git a/src/workerd/api/web-socket.h b/src/workerd/api/web-socket.h index 63ee58a1404..d0971bf961b 100644 --- a/src/workerd/api/web-socket.h +++ b/src/workerd/api/web-socket.h @@ -31,30 +31,39 @@ struct DeferredProxy; class CloseEvent: public Event { public: + // Runtime-only (the JS constructor uses the (type, ...) overload); always trusted. CloseEvent(uint code, kj::String reason, bool clean) - : Event("close"), + : Event("close", {}, Trusted::YES), code(code), reason(kj::mv(reason)), clean(clean) {} - CloseEvent(kj::String type, int code, kj::String reason, bool clean) - : Event(kj::mv(type)), + CloseEvent(kj::String type, int code, kj::String reason, bool clean, Init init = {}) + : Event(kj::mv(type), kj::mv(init)), code(code), reason(kj::mv(reason)), clean(clean) {} struct Initializer { + jsg::Optional bubbles; + jsg::Optional cancelable; + jsg::Optional composed; jsg::Optional code; jsg::Optional reason; jsg::Optional wasClean; - JSG_STRUCT(code, reason, wasClean); + JSG_STRUCT(bubbles, cancelable, composed, code, reason, wasClean); JSG_STRUCT_TS_OVERRIDE(CloseEventInit); }; static jsg::Ref constructor( jsg::Lock& js, kj::String type, jsg::Optional initializer) { Initializer init = kj::mv(initializer).orDefault({}); return js.alloc(kj::mv(type), init.code.orDefault(0), - kj::mv(init.reason).orDefault(jsg::USVString(kj::str())), init.wasClean.orDefault(false)); + kj::mv(init.reason).orDefault(jsg::USVString(kj::str())), init.wasClean.orDefault(false), + Event::Init{ + .bubbles = init.bubbles, + .cancelable = init.cancelable, + .composed = init.composed, + }); } int getCode() { @@ -176,6 +185,13 @@ class WebSocketPair: public jsg::Object { class WebSocketAdapter; +// WebSocket's UA-fired events are dispatched with spec semantics +// (DispatchExceptionPolicy::REPORT: listener exceptions are reported and the remaining +// listeners still run). For 'open' and 'message', a throwing listener additionally errors +// out the WebSocket after the dispatch completes (fail-fast), via +// DispatchResult::firstException; 'close' and 'error' fire when the WebSocket is already +// closed or failed and are report-only. See dispatchWithFailFast() and dispatchReportOnly() +// in web-socket.c++. class WebSocket: public EventTarget { public: // WebSocket ready states. diff --git a/src/workerd/io/compatibility-date.capnp b/src/workerd/io/compatibility-date.capnp index fb7b6a74516..0af4fd45a7e 100644 --- a/src/workerd/io/compatibility-date.capnp +++ b/src/workerd/io/compatibility-date.capnp @@ -1657,4 +1657,18 @@ struct CompatibilityFlags @0x8f8c1b68151b6cef { $experimental $pythonSnapshotRelease; # Enables Python Workers using Pyodide 314.0.5. + + specCompliantDispatchExceptions @188 :Bool + $compatEnableFlag("spec_compliant_dispatch_exceptions") + $compatDisableFlag("no_spec_compliant_dispatch_exceptions") + $compatEnableDate("2026-09-01"); + # Per the DOM spec, exceptions thrown by event listeners during dispatchEvent() should be + # reported (via the global 'error' event, then the console) but should not interrupt the + # dispatch or propagate to the dispatchEvent() caller. The original workerd implementation + # propagated the first listener exception and skipped remaining listeners for that event. + # + # When enabled, all event dispatch surfaces (the JS-visible dispatchEvent(), AbortSignal + # abort, and UA-fired events on WebSocket, EventSource, and MessagePort) use the spec's + # report-and-continue semantics. Internal runtime event delivery (fetch, scheduled, etc.) + # is not affected and always propagates. } diff --git a/src/workerd/io/external-pusher.c++ b/src/workerd/io/external-pusher.c++ index b9ad56354a2..5d7f8d8f974 100644 --- a/src/workerd/io/external-pusher.c++ +++ b/src/workerd/io/external-pusher.c++ @@ -146,7 +146,7 @@ namespace { class AbortTriggerRpcServer final: public rpc::AbortTrigger::Server { public: AbortTriggerRpcServer(kj::Own> fulfiller, - kj::Rc pendingReason) + kj::Arc pendingReason) : fulfiller(kj::mv(fulfiller)), pendingReason(kj::mv(pendingReason)) {} @@ -154,7 +154,7 @@ class AbortTriggerRpcServer final: public rpc::AbortTrigger::Server { auto params = abortCtx.getParams(); auto reason = params.getReason().getV8Serialized(); - *pendingReason = kj::heapArray(reason.asBytes()); + *pendingReason->value.lockExclusive() = kj::heapArray(reason.asBytes()); fulfiller->fulfill(); return kj::READY_NOW; } @@ -165,15 +165,18 @@ class AbortTriggerRpcServer final: public rpc::AbortTrigger::Server { } ~AbortTriggerRpcServer() noexcept(false) { - if (*pendingReason != nullptr) { - // Already triggered - return; - } + { + auto lock = pendingReason->value.lockExclusive(); + if (*lock != nullptr) { + // Already triggered + return; + } - if (!released) { - *pendingReason = JSG_KJ_EXCEPTION(FAILED, DOMAbortError, - "An AbortSignal received over RPC was implicitly aborted because the connection back to " - "its trigger was lost."); + if (!released) { + *lock = JSG_KJ_EXCEPTION(FAILED, DOMAbortError, + "An AbortSignal received over RPC was implicitly aborted because the connection back " + "to its trigger was lost."); + } } // Always fulfill the promise in case the AbortSignal was waiting @@ -182,7 +185,7 @@ class AbortTriggerRpcServer final: public rpc::AbortTrigger::Server { private: kj::Own> fulfiller; - kj::Rc pendingReason; + kj::Arc pendingReason; bool released = false; }; @@ -217,7 +220,7 @@ ExternalPusherImpl::AbortSignal ExternalPusherImpl::unwrapAbortSignal( // pushAbortSignal() might not have been received yet. So, we have to allocate the box here, so // we can return it. Then we can try to wire it up to the right trigger later, in // unwrapAbortSignalImpl(). - auto pendingReason = kj::rc(); + auto pendingReason = kj::arc(); auto promise = unwrapAbortSignalImpl(kj::mv(cap), pendingReason.addRef()); return { @@ -227,7 +230,7 @@ ExternalPusherImpl::AbortSignal ExternalPusherImpl::unwrapAbortSignal( } kj::Promise ExternalPusherImpl::unwrapAbortSignalImpl( - ExternalPusher::AbortSignal::Client cap, kj::Rc pendingReason) { + ExternalPusher::AbortSignal::Client cap, kj::Arc pendingReason) { auto paf = kj::newPromiseAndFulfiller(); { diff --git a/src/workerd/io/external-pusher.h b/src/workerd/io/external-pusher.h index 67a7b9e9640..11312f3e890 100644 --- a/src/workerd/io/external-pusher.h +++ b/src/workerd/io/external-pusher.h @@ -9,6 +9,7 @@ #include #include +#include namespace workerd { @@ -30,14 +31,24 @@ class ExternalPusherImpl: public rpc::JsValue::ExternalPusher::Server, public kj // Box which holds the reason why an AbortSignal was aborted. May be either: // - A serialized V8 value if the signal was aborted from JavaScript. // - A KJ exception if the connection from the trigger was lost. + // A pending abort reason received (or synthesized on disconnect) for an AbortSignal that + // was deserialized from RPC. A null OneOf means no abort has arrived yet. using PendingAbortReason = kj::OneOf, kj::Exception>; + // The box holding a PendingAbortReason. It is written at most once, from the receiving + // IoContext's thread, but may be read — under the mutex — from any thread: an AbortSignal + // that has crossed request boundaries polls it to answer getAborted()/getReason() + // synchronously everywhere. + struct PendingAbortReasonBox: public kj::AtomicRefcounted { + kj::MutexGuarded value; + }; + struct AbortSignal { // Resolves when `reason` has been filled in. kj::Promise signal; - // The abort reason box, will be uninitialized until `signal` resolves. - kj::Rc reason; + // The abort reason box, unfilled until `signal` resolves. + kj::Arc reason; }; AbortSignal unwrapAbortSignal(ExternalPusher::AbortSignal::Client cap); @@ -60,7 +71,7 @@ class ExternalPusherImpl: public rpc::JsValue::ExternalPusher::Server, public kj ExternalPusher::InputStream::Client cap); kj::Promise unwrapAbortSignalImpl( - ExternalPusher::AbortSignal::Client cap, kj::Rc pendingReason); + ExternalPusher::AbortSignal::Client cap, kj::Arc pendingReason); class InputStreamImpl; class AbortSignalImpl; diff --git a/src/workerd/io/io-context.h b/src/workerd/io/io-context.h index d1f41acf866..532229c7e0b 100644 --- a/src/workerd/io/io-context.h +++ b/src/workerd/io/io-context.h @@ -508,6 +508,13 @@ class IoContext final: public kj::Refcounted, private kj::TaskSet::ErrorHandler // Like requireCurrent() but throws a JS error if this IoContext is not the current. void requireCurrentOrThrowJs(); + // Returns an executor through which other IoContexts (or code running outside any + // IoContext) can later check whether this context is current, still alive, or defer work + // into it. Safe to retain beyond this context's lifetime. + IoCrossContextExecutor getCrossContextExecutor() { + return IoCrossContextExecutor(deleteQueue.queue.addRef()); + } + // A WeakRef is a weak reference to a IoContext. Note that because IoContext is not // itself ref-counted, we cannot follow the usual pattern of a weak reference that potentially // converts to a strong reference. Instead, intended usage looks like so: diff --git a/src/workerd/io/io-own.c++ b/src/workerd/io/io-own.c++ index bf1150e5d03..4e3860f5aa6 100644 --- a/src/workerd/io/io-own.c++ +++ b/src/workerd/io/io-own.c++ @@ -20,13 +20,8 @@ void DeleteQueue::scheduleDeletion(OwnedObject* object) const { } void DeleteQueue::scheduleAction(jsg::Lock& js, kj::Function&& action) const { - { - auto lock = crossThreadDeleteQueue.lockExclusive(); - KJ_IF_SOME(state, *lock) { - state.actions.add(kj::mv(action)); - KJ_REQUIRE_NONNULL(state.crossThreadFulfiller)->fulfill(); - return; - } + if (tryScheduleAction(kj::mv(action))) { + return; } // The queue was deleted, likely because the IoContext was destroyed and the @@ -54,6 +49,26 @@ void DeleteQueue::scheduleAction(jsg::Lock& js, kj::Function&& } } +bool DeleteQueue::tryScheduleAction(kj::Function&& action) const { + auto lock = crossThreadDeleteQueue.lockExclusive(); + KJ_IF_SOME(state, *lock) { + state.actions.add(kj::mv(action)); + KJ_REQUIRE_NONNULL(state.crossThreadFulfiller)->fulfill(); + return true; + } + // The queue was deleted, likely because the IoContext was destroyed and the DeleteQueuePtr + // was invalidated. The action is dropped on the floor. + return false; +} + +bool DeleteQueue::isCurrentIoContext() const { + return IoContext::hasCurrent() && IoContext::current().deleteQueue.queue.get() == this; +} + +bool DeleteQueue::isDefunct() const { + return *crossThreadDeleteQueue.lockShared() == kj::none; +} + void DeleteQueue::checkFarGet(const DeleteQueue& deleteQueue, const std::type_info& type) { IoContext::current().checkFarGet(deleteQueue, type); } @@ -110,4 +125,16 @@ void IoCrossContextExecutor::execute(jsg::Lock& js, kj::FunctionscheduleAction(js, kj::mv(func)); } +bool IoCrossContextExecutor::isCurrent() const { + return deleteQueue->isCurrentIoContext(); +} + +bool IoCrossContextExecutor::tryExecute(kj::Function&& func) const { + return deleteQueue->tryScheduleAction(kj::mv(func)); +} + +bool IoCrossContextExecutor::isTargetDestroyed() const { + return deleteQueue->isDefunct(); +} + } // namespace workerd diff --git a/src/workerd/io/io-own.h b/src/workerd/io/io-own.h index a7ad8c91121..b44155cb655 100644 --- a/src/workerd/io/io-own.h +++ b/src/workerd/io/io-own.h @@ -91,8 +91,25 @@ class DeleteQueue: public kj::AtomicRefcounted { DeleteQueue(): crossThreadDeleteQueue(State{kj::Vector()}) {} void scheduleDeletion(OwnedObject* object) const; + + // Schedules the given action to run in the IoContext that owns this queue, the next time + // that context drains its queue (in its own thread, under the isolate lock). If the owning + // IoContext has already been destroyed, the action is dropped and a warning about + // cross-request promise resolution is logged to the current context, if any. void scheduleAction(jsg::Lock& js, kj::Function&& action) const; + // Like scheduleAction(), but for callers where a destroyed target context is an expected, + // benign outcome: returns true if the action was queued, or false — dropping the action + // silently — if the owning IoContext has already been destroyed. + bool tryScheduleAction(kj::Function&& action) const; + + // True if the IoContext that owns this queue is the calling thread's current IoContext. + bool isCurrentIoContext() const; + + // True if the IoContext that owns this queue has been destroyed, meaning scheduled actions + // and deletions are dropped. Useful for reclaiming registrations that target this queue. + bool isDefunct() const; + struct State { kj::Vector queue; // Actions that some other IoContext has requested be executed in this IoContext. When @@ -145,6 +162,19 @@ class IoCrossContextExecutor { // The target IoContext will be signaled to run the action as soon as it is able. void execute(jsg::Lock& js, kj::Function&& action); + // True if the IoContext this executor targets is the calling thread's current IoContext — + // i.e. the caller could run the work synchronously instead of deferring through + // tryExecute(). + bool isCurrent() const; + + // Like execute(), but for callers where a destroyed target context is an expected, benign + // outcome: returns true if the action was queued, or false — dropping the action silently — + // if the target IoContext has already been destroyed. + bool tryExecute(kj::Function&& action) const; + + // True if the target IoContext has been destroyed, i.e. tryExecute() would drop the action. + bool isTargetDestroyed() const; + private: friend class IoContext; friend class DeleteQueue; diff --git a/src/workerd/server/tests/container-client/container-client.wd-test b/src/workerd/server/tests/container-client/container-client.wd-test index 776d6bc7841..78504e3de86 100644 --- a/src/workerd/server/tests/container-client/container-client.wd-test +++ b/src/workerd/server/tests/container-client/container-client.wd-test @@ -8,7 +8,7 @@ const unitTests :Workerd.Config = ( modules = [ (name = "worker", esModule = embed "test.js") ], - compatibilityFlags = ["enable_ctx_exports", "nodejs_compat", "experimental", "containers_pid_namespace", "streams_enable_constructors"], + compatibilityFlags = ["enable_ctx_exports", "nodejs_compat", "experimental", "containers_pid_namespace", "streams_enable_constructors", "enable_abortsignal_rpc"], containerEngine = (localDocker = (socketPath = "unix:/var/run/docker.sock", containerEgressInterceptorImage = "cloudflare/proxy-everything:main")), durableObjectNamespaces = [ ( className = "DurableObjectExample", diff --git a/src/workerd/server/tests/container-client/test.js b/src/workerd/server/tests/container-client/test.js index 6ff41de07fb..dbac16ebbf9 100644 --- a/src/workerd/server/tests/container-client/test.js +++ b/src/workerd/server/tests/container-client/test.js @@ -400,6 +400,38 @@ export class DurableObjectExample extends DurableObject { assert.strictEqual(container.running, false); } + // Runs a long-lived process wired to an AbortSignal that this Durable Object received over + // RPC (from the test driver) and returns the process exit code. This exercises exec()'s + // abort registration against a *deserialized* signal: the registration itself must arm the + // signal's RPC abort subscription, or the remote abort would never be delivered here. + async execWithReceivedSignal(signal) { + const container = this.ctx.container; + if (!container.running) { + container.start(); + } + const monitor = container.monitor().catch((_err) => {}); + await this.waitUntilContainerIsHealthy(); + + const proc = await container.exec(['sh', '-lc', 'sleep 60'], { + signal, + stdout: 'ignore', + }); + this.#receivedSignalExecStarted = true; + const exitCode = await proc.exitCode; + + await container.destroy(); + await monitor; + return exitCode; + } + + // Polled by the test driver so it only aborts once the exec is actually running (aborting + // earlier would make exec() itself fail fast instead of killing the process). + async receivedSignalExecStarted() { + return this.#receivedSignalExecStarted; + } + + #receivedSignalExecStarted = false; + async testSetInactivityTimeout(timeout) { const container = this.ctx.container; if (container.running) { @@ -2897,6 +2929,27 @@ export const testExec = { }, }; +// An AbortSignal passed into the Durable Object over RPC kills an exec()'d process when +// aborted from the caller's context. +export const testExecRemoteAbortSignal = { + async test(_ctrl, env) { + const id = env.MY_CONTAINER.idFromName( + getRandomDurableObjectName('testExecRemoteAbortSignal') + ); + const stub = env.MY_CONTAINER.get(id); + + const ac = new AbortController(); + const pending = stub.execWithReceivedSignal(ac.signal); + while (!(await stub.receivedSignalExecStarted())) { + await scheduler.wait(100); + } + ac.abort(new Error('remote-abort')); + + // A process killed by SIGKILL (9) reports exit code 128 + 9 = 137. + assert.strictEqual(await pending, 137); + }, +}; + // Test exit code monitor functionality export const testExitCode = { async test(_ctrl, env) { diff --git a/src/workerd/util/BUILD.bazel b/src/workerd/util/BUILD.bazel index b902f621d26..4ab9713aeb0 100644 --- a/src/workerd/util/BUILD.bazel +++ b/src/workerd/util/BUILD.bazel @@ -65,6 +65,7 @@ wd_cc_library( wd_cc_library( name = "util", srcs = [ + "canceler.c++", "stream-utils.c++", "wait-list.c++", ], @@ -383,6 +384,13 @@ kj_test( ], ) +kj_test( + src = "canceler-test.c++", + deps = [ + ":util", + ], +) + kj_test( src = "mimetype-test.c++", deps = [ diff --git a/src/workerd/util/abortable.h b/src/workerd/util/abortable.h index 32ce3b68623..8c876803b18 100644 --- a/src/workerd/util/abortable.h +++ b/src/workerd/util/abortable.h @@ -12,8 +12,14 @@ namespace workerd { template class AbortableImpl final { public: - AbortableImpl(kj::Own inner, RefcountedCanceler& canceler) - : canceler(kj::addRef(canceler)), + // In addition to the canceler itself, the caller may pass an opaque registration handle + // that keeps the canceler hooked up to whatever triggers it (e.g. an AbortSignal); it is + // held only so that it is dropped when this object is destroyed. + AbortableImpl(kj::Own inner, + kj::Own canceler, + kj::Own cancelerRegistration = kj::Own()) + : canceler(kj::mv(canceler)), + cancelerRegistration(kj::mv(cancelerRegistration)), inner(kj::mv(inner)), onCancel(*(this->canceler), [this]() { this->inner = kj::none; }) {} @@ -42,23 +48,28 @@ class AbortableImpl final { } private: - kj::Own canceler; + kj::Own canceler; + kj::Own cancelerRegistration; kj::Maybe> inner; - RefcountedCanceler::Listener onCancel; + // Must be declared after `canceler` so that the listener unregisters itself while the + // canceler is still alive. + ReleasingCanceler::Listener onCancel; }; -// An InputStream that can be disconnected in response to RefcountedCanceler. +// An InputStream that can be disconnected in response to ReleasingCanceler. // This is similar to NeuterableInputStream in global-scope.c++ but uses an // external kj::Canceler to trigger the disconnect. // This is currently only used in fetch() requests that use an AbortSignal. -// The AbortableInputStream is created using a RefcountedCanceler, +// The AbortableInputStream is created using a ReleasingCanceler, // which will be triggered when the AbortSignal is triggered. // TODO(later): It would be good to see if both this and NeuterableInputStream // could be combined into a single utility. class AbortableInputStream final: public kj::AsyncInputStream, public kj::Refcounted { public: - AbortableInputStream(kj::Own inner, RefcountedCanceler& canceler) - : impl(kj::mv(inner), canceler) {} + AbortableInputStream(kj::Own inner, + kj::Own canceler, + kj::Own cancelerRegistration = kj::Own()) + : impl(kj::mv(inner), kj::mv(canceler), kj::mv(cancelerRegistration)) {} kj::Promise tryRead(void* buffer, size_t minBytes, size_t maxBytes) override { kj::Promise (kj::AsyncInputStream::*tryRead)(void*, size_t, size_t) = @@ -78,14 +89,16 @@ class AbortableInputStream final: public kj::AsyncInputStream, public kj::Refcou AbortableImpl impl; }; -// A WebSocket wrapper that can be disconnected in response to a RefcountedCanceler. +// A WebSocket wrapper that can be disconnected in response to a ReleasingCanceler. // This is currently only used when opening a WebSocket with a fetch() request that // is using an AbortSignal. The AbortableWebSocket is created using the AbortSignal's -// RefcountedCanceler, which will be triggered when the AbortSignal is triggered. +// ReleasingCanceler, which will be triggered when the AbortSignal is triggered. class AbortableWebSocket final: public kj::WebSocket, public kj::Refcounted { public: - AbortableWebSocket(kj::Own inner, RefcountedCanceler& canceler) - : impl(kj::mv(inner), canceler) {} + AbortableWebSocket(kj::Own inner, + kj::Own canceler, + kj::Own cancelerRegistration = kj::Own()) + : impl(kj::mv(inner), kj::mv(canceler), kj::mv(cancelerRegistration)) {} kj::Promise send(kj::ArrayPtr message) override { return impl.wrap( diff --git a/src/workerd/util/canceler-test.c++ b/src/workerd/util/canceler-test.c++ new file mode 100644 index 00000000000..9eef86e416c --- /dev/null +++ b/src/workerd/util/canceler-test.c++ @@ -0,0 +1,154 @@ +// Copyright (c) 2017-2022 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 + +#include "canceler.h" + +#include +#include + +namespace workerd { +namespace { + +kj::Exception testException() { + return KJ_EXCEPTION(DISCONNECTED, "canceled for testing"); +} + +KJ_TEST("ReleasingCanceler cancels wrapped promises with the given exception") { + kj::EventLoop loop; + kj::WaitScope ws(loop); + + ReleasingCanceler canceler; + KJ_EXPECT(!canceler.isCanceled()); + + auto promise = canceler.wrap(kj::Promise(kj::NEVER_DONE)); + + canceler.cancel(testException()); + KJ_EXPECT(canceler.isCanceled()); + KJ_EXPECT_THROW_MESSAGE("canceled for testing", promise.wait(ws)); + KJ_EXPECT_THROW_MESSAGE("canceled for testing", canceler.throwIfCanceled()); +} + +KJ_TEST("ReleasingCanceler wrap after cancellation rejects immediately") { + kj::EventLoop loop; + kj::WaitScope ws(loop); + + ReleasingCanceler canceler; + canceler.cancel(testException()); + + auto promise = canceler.wrap(kj::Promise(kj::READY_NOW)); + KJ_EXPECT_THROW_MESSAGE("canceled for testing", promise.wait(ws)); +} + +KJ_TEST("ReleasingCanceler constructed pre-canceled behaves as canceled") { + kj::EventLoop loop; + kj::WaitScope ws(loop); + + ReleasingCanceler canceler(testException()); + KJ_EXPECT(canceler.isCanceled()); + + auto promise = canceler.wrap(kj::Promise(kj::READY_NOW)); + KJ_EXPECT_THROW_MESSAGE("canceled for testing", promise.wait(ws)); +} + +KJ_TEST("ReleasingCanceler releases (does not cancel) wrapped promises on drop") { + kj::EventLoop loop; + kj::WaitScope ws(loop); + + auto paf = kj::newPromiseAndFulfiller(); + kj::Promise wrapped = nullptr; + { + ReleasingCanceler canceler; + wrapped = canceler.wrap(kj::mv(paf.promise)); + } + + // The canceler is gone but the wrapped promise was released, not canceled: it still + // completes through its original path. + KJ_EXPECT(!wrapped.poll(ws)); + paf.fulfiller->fulfill(123); + KJ_EXPECT(wrapped.wait(ws) == 123); +} + +KJ_TEST("ReleasingCanceler fires listeners exactly once on cancellation") { + ReleasingCanceler canceler; + + int fired = 0; + ReleasingCanceler::Listener listener(canceler, [&fired]() { ++fired; }); + KJ_EXPECT(fired == 0); + + canceler.cancel(testException()); + KJ_EXPECT(fired == 1); + + // A second cancellation is a no-op. + canceler.cancel(KJ_EXCEPTION(FAILED, "some other reason")); + KJ_EXPECT(fired == 1); + KJ_EXPECT_THROW_MESSAGE("canceled for testing", canceler.throwIfCanceled()); +} + +KJ_TEST("ReleasingCanceler fires a late listener immediately and never links it") { + ReleasingCanceler canceler; + canceler.cancel(testException()); + + int fired = 0; + ReleasingCanceler::Listener listener(canceler, [&fired]() { ++fired; }); + KJ_EXPECT(fired == 1); +} + +KJ_TEST("ReleasingCanceler does not fire a listener destroyed before cancellation") { + ReleasingCanceler canceler; + + int fired = 0; + { + ReleasingCanceler::Listener listener(canceler, [&fired]() { ++fired; }); + } + + canceler.cancel(testException()); + KJ_EXPECT(fired == 0); +} + +KJ_TEST("ReleasingCanceler listeners may outlive the canceler once fired") { + int fired = 0; + kj::Maybe listener; + { + ReleasingCanceler canceler; + listener.emplace(canceler, [&fired]() { ++fired; }); + canceler.cancel(testException()); + KJ_EXPECT(fired == 1); + // The canceler is destroyed here, before the (already fired, and therefore unlinked) + // listener; the listener's destructor must not touch it. + } + listener = kj::none; +} + +KJ_TEST("ReleasingCanceler listener callbacks may destroy other listeners") { + ReleasingCanceler canceler; + + int fired = 0; + kj::Maybe second; + ReleasingCanceler::Listener first(canceler, [&second]() { second = kj::none; }); + second.emplace(canceler, [&fired]() { ++fired; }); + + // The first listener destroys the second while the cancellation is being delivered; the + // second must simply not fire. + canceler.cancel(testException()); + KJ_EXPECT(fired == 0); +} + +KJ_TEST("ReleasingCanceler listener callbacks may register new listeners") { + ReleasingCanceler canceler; + + // A listener registered from within a callback observes the already-canceled state: it + // fires immediately (and is never linked) rather than being picked up by the drain. + int fired = 0; + kj::Maybe late; + ReleasingCanceler::Listener first(canceler, [&canceler, &late, &fired]() { + late.emplace(canceler, [&fired]() { ++fired; }); + KJ_EXPECT(fired == 1); + }); + + canceler.cancel(testException()); + KJ_EXPECT(fired == 1); +} + +} // namespace +} // namespace workerd diff --git a/src/workerd/util/canceler.c++ b/src/workerd/util/canceler.c++ new file mode 100644 index 00000000000..77adbcb09bf --- /dev/null +++ b/src/workerd/util/canceler.c++ @@ -0,0 +1,83 @@ +// Copyright (c) 2017-2022 Cloudflare, Inc. +// Licensed under the Apache 2.0 license found in the LICENSE file or at: +// https://opensource.org/licenses/Apache-2.0 + +#include "canceler.h" + +namespace workerd { + +ReleasingCanceler::Listener::Listener(ReleasingCanceler& canceler, kj::Function fn) + : fn(kj::mv(fn)), + canceler(canceler) { + canceler.addListener(*this); +} + +ReleasingCanceler::Listener::~Listener() noexcept(false) { + if (link.isLinked()) { + canceler.removeListener(*this); + } +} + +ReleasingCanceler::ReleasingCanceler(kj::Maybe reason): reason(kj::mv(reason)) {} + +ReleasingCanceler::~ReleasingCanceler() noexcept(false) { + // `listeners` only contains listeners still awaiting cancellation, and those must not + // outlive the canceler. (Listeners that already fired were unlinked and may live on.) + KJ_ASSERT(listeners.empty()); + + // Release rather than cancel any remaining wrapped promises: dropping the canceler + // without an explicit cancellation must not reject the wrapped work. + canceler.release(); +} + +void ReleasingCanceler::cancel(const kj::Exception& exception) { + if (reason == kj::none) { + reason = exception.clone(); + canceler.cancel(exception); + + // Drain the list, unlinking each listener before invoking it: a fired listener can never + // fire again, so there is no reason to retain it, and once unlinked its lifetime is + // decoupled from this canceler (its destructor no longer needs to touch us). Unlinking + // first also makes it safe for a callback to destroy its own — or any other — listener. + // + // Re-entrancy (mirroring kj::Canceler::cancel()'s unlink-then-invoke drain): a callback + // may call cancel() again (no-op, `reason` is already set), register new listeners (they + // fire immediately without linking, see addListener()), or destroy pending listeners + // (their destructors self-remove from the live list, which is re-consulted on every + // iteration). It is the callback's responsibility NOT to destroy the canceler itself, + // and not to throw. + while (!listeners.empty()) { + auto& listener = listeners.front(); + listeners.remove(listener); + listener.fn(); + } + } +} + +void ReleasingCanceler::throwIfCanceled() { + KJ_IF_SOME(ex, reason) { + kj::throwFatalException(ex.clone()); + } +} + +bool ReleasingCanceler::isCanceled() const { + return reason != kj::none; +} + +void ReleasingCanceler::addListener(Listener& listener) { + if (reason != kj::none) { + // The canceler was already canceled; a listener registered now would otherwise never be + // notified. Fire it immediately — without ever linking it, since it cannot fire again — + // so that late registrants observe cancellation the same way wrap() does (which returns + // the exception immediately). + listener.fn(); + return; + } + listeners.add(listener); +} + +void ReleasingCanceler::removeListener(Listener& listener) { + listeners.remove(listener); +} + +} // namespace workerd diff --git a/src/workerd/util/canceler.h b/src/workerd/util/canceler.h index fa201efdfe3..d4399d82446 100644 --- a/src/workerd/util/canceler.h +++ b/src/workerd/util/canceler.h @@ -12,47 +12,47 @@ namespace workerd { -// A simple wrapper around kj::Canceler that can be safely -// shared by multiple objects. This is used, for instance, -// to support fetch() requests that use an AbortSignal. -// The AbortSignal (see api/basics.h) creates an instance -// of RefcountedCanceler then passes references to it out -// to various other objects that will use it to wrap their -// Promises. -class RefcountedCanceler: public kj::Refcounted { +// A canceler that combines a kj::Canceler with sticky cancellation state and observer +// callbacks and (unlike kj::Canceler, whose destructor implicitly cancels) releases +// any still-wrapped promises when dropped without having been canceled. +// +// This is used, for instance, to support fetch() requests that use an AbortSignal: +// the signal's abort registration cancels it (via a reference whose validity the +// registration's RAII handle guarantees; see api::AbortSignal), while wrappers like +// AbortableInputStream wrap their promises through it and observe cancellation through +// Listener. +class ReleasingCanceler final { public: - class Listener { + // Invokes fn when the canceler is canceled. If the canceler was ALREADY canceled at + // registration time, fn is invoked immediately. The fn is invoked at most once. + // + // A listener is only linked to the canceler while it is still awaiting cancellation: once + // it has fired (or if it registered after cancellation), it no longer references the + // canceler and may safely outlive it. A listener that has NOT yet fired must be destroyed + // before the canceler. + class Listener final { public: - explicit Listener(RefcountedCanceler& canceler, kj::Function fn) - : fn(kj::mv(fn)), - canceler(canceler) { - canceler.addListener(*this); - } + explicit Listener(ReleasingCanceler& canceler, kj::Function fn); + ~Listener() noexcept(false); - ~Listener() { - canceler.removeListener(*this); - } + // Also implied by the ListLink member (and the reference member, for assignment), but + // stated explicitly for clarity and better diagnostics: a linked Listener's address must + // remain stable, and it must not outlive its canceler. + KJ_DISALLOW_COPY_AND_MOVE(Listener); private: kj::Function fn; - RefcountedCanceler& canceler; + ReleasingCanceler& canceler; kj::ListLink link; - friend class RefcountedCanceler; + friend class ReleasingCanceler; }; - RefcountedCanceler(kj::Maybe reason = kj::none): reason(kj::mv(reason)) {} + ReleasingCanceler(kj::Maybe reason = kj::none); - ~RefcountedCanceler() noexcept(false) { - // `listeners` has to be empty since each listener should have held a strong reference. - KJ_ASSERT(listeners.empty()); + ~ReleasingCanceler() noexcept(false); - // RefcountedCanceler is used in use cases where we don't want to cancel by default if the - // canceler is destroyed, so release any remaining wrapped promises. - canceler.release(); - } - - KJ_DISALLOW_COPY_AND_MOVE(RefcountedCanceler); + KJ_DISALLOW_COPY_AND_MOVE(ReleasingCanceler); template kj::Promise wrap(kj::Promise promise) { @@ -62,49 +62,19 @@ class RefcountedCanceler: public kj::Refcounted { return canceler.wrap(kj::mv(promise)); } - void cancel(kj::StringPtr cancelReason) { - if (reason == kj::none) { - cancel(kj::Exception( - kj::Exception::Type::DISCONNECTED, __FILE__, __LINE__, kj::str(cancelReason))); - } - } - - void cancel(const kj::Exception& exception) { - if (reason == kj::none) { - reason = exception.clone(); - canceler.cancel(exception); - for (auto& listener: listeners) { - listener.fn(); - } - } - } - - bool isEmpty() const { - return canceler.isEmpty(); - } - - void throwIfCanceled() { - KJ_IF_SOME(ex, reason) { - kj::throwFatalException(ex.clone()); - } - } + void cancel(const kj::Exception& exception); - bool isCanceled() const { - return reason != kj::none; - } + void throwIfCanceled(); - void addListener(Listener& listener) { - listeners.add(listener); - } - - void removeListener(Listener& listener) { - listeners.remove(listener); - } + bool isCanceled() const; private: kj::Canceler canceler; kj::Maybe reason; + void addListener(Listener& listener); + void removeListener(Listener& listener); + kj::List listeners; }; diff --git a/src/wpt/dom/abort-test.ts b/src/wpt/dom/abort-test.ts index 5b207cae086..d40cac11a8a 100644 --- a/src/wpt/dom/abort-test.ts +++ b/src/wpt/dom/abort-test.ts @@ -6,12 +6,7 @@ import { type TestRunnerConfig } from 'harness/harness'; export default { 'AbortSignal.any.js': {}, - 'abort-signal-any.any.js': { - comment: 'Order of event firing should be investigated.', - expectedFailures: [ - 'Abort events for AbortSignal.any() signals fire in the right order (using AbortController)', - ], - }, + 'abort-signal-any.any.js': {}, 'event.any.js': {}, 'timeout.any.js': {}, } satisfies TestRunnerConfig; diff --git a/tools/base.eslint.config.mjs b/tools/base.eslint.config.mjs index 31c4ffe162c..eb786b3d026 100644 --- a/tools/base.eslint.config.mjs +++ b/tools/base.eslint.config.mjs @@ -18,6 +18,7 @@ const workerdGlobals = { CustomEvent: 'readonly', DecompressionStream: 'readonly', DOMException: 'readonly', + ErrorEvent: 'readonly', Event: 'readonly', EventSource: 'readonly', EventTarget: 'readonly', diff --git a/types/generated-snapshot/experimental/index.d.ts b/types/generated-snapshot/experimental/index.d.ts index 768fd5206e2..123142b97cc 100755 --- a/types/generated-snapshot/experimental/index.d.ts +++ b/types/generated-snapshot/experimental/index.d.ts @@ -1691,6 +1691,9 @@ declare class ErrorEvent extends Event { get error(): any; } interface ErrorEventErrorEventInit { + bubbles?: boolean; + cancelable?: boolean; + composed?: boolean; message?: string; filename?: string; lineno?: number; @@ -1703,7 +1706,7 @@ interface ErrorEventErrorEventInit { * [MDN Reference](https://developer.mozilla.org/docs/Web/API/MessageEvent) */ declare class MessageEvent extends Event { - constructor(type: string, initializer: MessageEventInit); + constructor(type: string, initializer?: MessageEventInit); /** * The **`data`** read-only property of the MessageEvent interface represents the data sent by the message emitter. * @@ -1736,7 +1739,14 @@ declare class MessageEvent extends Event { readonly ports: MessagePort[]; } interface MessageEventInit { - data: ArrayBuffer | string; + bubbles?: boolean; + cancelable?: boolean; + composed?: boolean; + data?: any; + origin?: string; + lastEventId?: string; + source?: MessagePort; + ports?: MessagePort[]; } /** * The **`PromiseRejectionEvent`** interface represents events which are sent to the global script context when JavaScript Promises are rejected. These events are particularly useful for telemetry and debugging purposes. @@ -3805,6 +3815,9 @@ declare class CloseEvent extends Event { readonly wasClean: boolean; } interface CloseEventInit { + bubbles?: boolean; + cancelable?: boolean; + composed?: boolean; code?: number; reason?: string; wasClean?: boolean; diff --git a/types/generated-snapshot/experimental/index.ts b/types/generated-snapshot/experimental/index.ts index 13f54e10e9d..2b17f950bc2 100755 --- a/types/generated-snapshot/experimental/index.ts +++ b/types/generated-snapshot/experimental/index.ts @@ -1695,6 +1695,9 @@ export declare class ErrorEvent extends Event { get error(): any; } export interface ErrorEventErrorEventInit { + bubbles?: boolean; + cancelable?: boolean; + composed?: boolean; message?: string; filename?: string; lineno?: number; @@ -1707,7 +1710,7 @@ export interface ErrorEventErrorEventInit { * [MDN Reference](https://developer.mozilla.org/docs/Web/API/MessageEvent) */ export declare class MessageEvent extends Event { - constructor(type: string, initializer: MessageEventInit); + constructor(type: string, initializer?: MessageEventInit); /** * The **`data`** read-only property of the MessageEvent interface represents the data sent by the message emitter. * @@ -1740,7 +1743,14 @@ export declare class MessageEvent extends Event { readonly ports: MessagePort[]; } export interface MessageEventInit { - data: ArrayBuffer | string; + bubbles?: boolean; + cancelable?: boolean; + composed?: boolean; + data?: any; + origin?: string; + lastEventId?: string; + source?: MessagePort; + ports?: MessagePort[]; } /** * The **`PromiseRejectionEvent`** interface represents events which are sent to the global script context when JavaScript Promises are rejected. These events are particularly useful for telemetry and debugging purposes. @@ -3814,6 +3824,9 @@ export declare class CloseEvent extends Event { readonly wasClean: boolean; } export interface CloseEventInit { + bubbles?: boolean; + cancelable?: boolean; + composed?: boolean; code?: number; reason?: string; wasClean?: boolean; diff --git a/types/generated-snapshot/index.d.ts b/types/generated-snapshot/index.d.ts index a67b42aea2e..e6902aba405 100755 --- a/types/generated-snapshot/index.d.ts +++ b/types/generated-snapshot/index.d.ts @@ -1666,6 +1666,9 @@ declare class ErrorEvent extends Event { get error(): any; } interface ErrorEventErrorEventInit { + bubbles?: boolean; + cancelable?: boolean; + composed?: boolean; message?: string; filename?: string; lineno?: number; @@ -1678,7 +1681,7 @@ interface ErrorEventErrorEventInit { * [MDN Reference](https://developer.mozilla.org/docs/Web/API/MessageEvent) */ declare class MessageEvent extends Event { - constructor(type: string, initializer: MessageEventInit); + constructor(type: string, initializer?: MessageEventInit); /** * The **`data`** read-only property of the MessageEvent interface represents the data sent by the message emitter. * @@ -1711,7 +1714,14 @@ declare class MessageEvent extends Event { readonly ports: MessagePort[]; } interface MessageEventInit { - data: ArrayBuffer | string; + bubbles?: boolean; + cancelable?: boolean; + composed?: boolean; + data?: any; + origin?: string; + lastEventId?: string; + source?: MessagePort; + ports?: MessagePort[]; } /** * The **`PromiseRejectionEvent`** interface represents events which are sent to the global script context when JavaScript Promises are rejected. These events are particularly useful for telemetry and debugging purposes. @@ -3728,6 +3738,9 @@ declare class CloseEvent extends Event { readonly wasClean: boolean; } interface CloseEventInit { + bubbles?: boolean; + cancelable?: boolean; + composed?: boolean; code?: number; reason?: string; wasClean?: boolean; diff --git a/types/generated-snapshot/index.ts b/types/generated-snapshot/index.ts index c3d5b9f18cc..f6b7657a6fe 100755 --- a/types/generated-snapshot/index.ts +++ b/types/generated-snapshot/index.ts @@ -1670,6 +1670,9 @@ export declare class ErrorEvent extends Event { get error(): any; } export interface ErrorEventErrorEventInit { + bubbles?: boolean; + cancelable?: boolean; + composed?: boolean; message?: string; filename?: string; lineno?: number; @@ -1682,7 +1685,7 @@ export interface ErrorEventErrorEventInit { * [MDN Reference](https://developer.mozilla.org/docs/Web/API/MessageEvent) */ export declare class MessageEvent extends Event { - constructor(type: string, initializer: MessageEventInit); + constructor(type: string, initializer?: MessageEventInit); /** * The **`data`** read-only property of the MessageEvent interface represents the data sent by the message emitter. * @@ -1715,7 +1718,14 @@ export declare class MessageEvent extends Event { readonly ports: MessagePort[]; } export interface MessageEventInit { - data: ArrayBuffer | string; + bubbles?: boolean; + cancelable?: boolean; + composed?: boolean; + data?: any; + origin?: string; + lastEventId?: string; + source?: MessagePort; + ports?: MessagePort[]; } /** * The **`PromiseRejectionEvent`** interface represents events which are sent to the global script context when JavaScript Promises are rejected. These events are particularly useful for telemetry and debugging purposes. @@ -3737,6 +3747,9 @@ export declare class CloseEvent extends Event { readonly wasClean: boolean; } export interface CloseEventInit { + bubbles?: boolean; + cancelable?: boolean; + composed?: boolean; code?: number; reason?: string; wasClean?: boolean;