Skip to content

fix(sdk): release completed embedded request signals - #53613

Merged
kitlangton merged 2 commits into
v2from
release-request-signals
Oct 6, 2026
Merged

kitlangton merged 2 commits into
v2from
release-request-signals

Conversation

@kitlangton

@kitlangton kitlangton commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Why

Long-lived embedded SDK hosts leak every completed Request, Headers, AbortSignal, and request body buffer until close() is called. OwnedFetch.make previously bound each request to AbortSignal.any([source.signal, shutdown.signal]), and Effect's web handler (HttpEffect.toWebHandler*) registers an abort listener on request.signal without removing it when the request completes normally. Because shutdown.signal stayed live for the lifetime of the host, the dependent signal and its abort listener remained reachable after response consumption and GC.

What Changes

Before:

  • OwnedFetch.make kept a host-lifetime shutdown AbortController and constructed AbortSignal.any([source.signal, shutdown.signal]) for every request.
  • Even after the response body finished or the handler rejected, shutdown.signal retained the dependent request.signal and the Effect web handler's Request closure for the lifetime of the host (200 client.server.info() calls retained 201 Request instances after GC).

After:

  • OwnedFetch.make tracks active requests in Map<AbortController, Promise<void>> instead of maintaining a host-level shutdown AbortController.
  • Each request forwards source.signal aborts to its per-request controller and detaches that listener in finish() when the response completes, cancels, or rejects.
  • close() aborts only the active controllers currently in requests and awaits their lifetimes before disposing the runtime.
sequenceDiagram
  participant Caller
  participant OwnedFetch
  participant Handler as Effect Web Handler

  Caller->>OwnedFetch: fetch(input, init)
  OwnedFetch->>OwnedFetch: requests.set(controller, lifetime.promise)
  OwnedFetch->>Handler: handler(request with controller.signal)
  Handler-->>OwnedFetch: Response
  Caller->>OwnedFetch: consume / cancel response body
  OwnedFetch->>OwnedFetch: finish() detaches source.signal & deletes controller
Loading

Scope

This PR updates OwnedFetch.make in packages/sdk/src/internal/fetch.ts and adds embedded request-retention and cancellation tests in packages/sdk/test/request-retention.test.ts.

Verification

bun test test/request-retention.test.ts   # in packages/sdk
bun typecheck                             # in packages/sdk
bun run check
  • test/request-retention.test.ts: Failed on origin/v2 with 201 retained Request objects after 200 client.server.info() calls and 120/120 retained Request objects on the failure path; all 4 tests pass with this change.
  • bun typecheck in packages/sdk and repo-wide bun run check pass.

@kitlangton
kitlangton merged commit 9ee97d8 into v2 Oct 6, 2026
11 checks passed
@kitlangton
kitlangton deleted the release-request-signals branch October 6, 2026 22:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant