cloudflare / cloudflare/workerd

🐛 Bug Report — Runtime APIs: client disconnect no longer cancels async ReadableStream response body (regressed in 1.20260619.1, persists in 1.20260717.1)

Open
#6,832 1 comment 0 reactions 0 assignees View on GitHub
Dominant language
C++
Stars
8.7k
Forks
739
Avg merge
2d 20h
Merged PRs (30d)
174

Description

`workerd@1.20260619.1` regresses client-disconnect handling for async JavaScript `ReadableStream` response bodies, and the regression is still present in the current release (`1.20260717.1`).

**TL;DR:** when the client disconnects mid-stream, the JS source's `cancel()` algorithm no longer runs. With `enable_request_signal` opted in, `request.signal` still aborts on every affected version — the disconnect demonstrably reaches the worker; only the stream-cancel wiring was lost. That points at dropped wiring rather than an intentional behavior change. (An earlier revision of this issue reported `request.signal` as not firing either; that observation was made without the `enable_request_signal` flag, which has no default enable date, so the signal never fires by design in that configuration.)

Minimal repro:

https://github.com/cnluzhang/workerd-stream-cancel-repro

```bash
git clone git@github.com:cnluzhang/workerd-stream-cancel-repro.git
cd workerd-stream-cancel-repro
npm install
npm test
```

The repro runs a 3-version x 2-topology matrix — `direct` (client socket terminates on the streaming worker's own HTTP listener) and `proxied` (a proxy worker forwards over a service binding, matching production topologies with an ingress hop) — with `enable_request_signal` opted in:

```text
- workerd@1.20260617.1 [direct]: stream=cancel, signal=aborted
- workerd@1.20260617.1 [proxied]: stream=cancel, signal=aborted
- workerd@1.20260619.1 [direct]: stream=ended-normally, signal=aborted
- workerd@1.20260619.1 [proxied]: stream=ended-normally, signal=aborted
- workerd@1.20260717.1 [direct]: stream=ended-normally, signal=aborted
- workerd@1.20260717.1 [proxied]: stream=ended-normally, signal=aborted
```

`signal=aborted` in every cell shows the disconnect reaches the worker on all three versions and both topologies. Only the stream leg regressed: from `1.20260619.1` on, the source's `cancel()` never runs, later `controller.enqueue()` calls do not throw, and the async stream runs to natural completion after the client is gone.

The repro worker returns an async stream:

```js
return new Response(new ReadableStream({
async start(controller) {
controller.enqueue(new TextEncoder().encode("first\n"));
for (let i = 0; i < 20; i += 1) {
await new Promise((resolve) => setTimeout(resolve, 100));
try {
controller.enqueue(new TextEncoder().encode(`tick:${i}\n`));
} catch {
record.stream = "enqueue-threw";
return;
}
}
record.stream = "ended-normally";
},
cancel() {
record.stream = "cancel";
},
}));
```

The client reads the first chunk and calls `res.socket.destroy()` (abortive close). That is the path kj's `HttpServer` reliably notices mid-response, dropping the response-body pump. Gentler client teardown may not be observed by the server at all (see the `TODO(perf): If the client disconnects, should we cancel the response?` comment in kj's `HttpServer::Connection`), so a non-repro with a different client teardown does not invalidate this report.

## Root cause (confirmed by code inspection)

Commit `c32933263` ("Remove draining read standard streams autogate") deleted the old default `PumpToReader` path. Its teardown was the only code that translated a dropped response-body pump into the stream's JS `cancel()` algorithm: when the kj promise holding the pump was dropped, the still-pending JS continuation found the `WeakRef` invalidated and called `readable->getController().cancel(js, kj::none)`.

The replacement path has no equivalent:

- `ReadableStreamJsController::pumpTo()` now always uses `DrainingReader` / `pumpToImpl` (`src/workerd/api/streams/standard.c++`).
- `pumpToImpl` invokes `reader->cancel()` only from its error (`KJ_CATCH`) path. In this repro the stream is async and usually suspended awaiting the next chunk when the client disconnects, so there is no `sink->write()` exception to enter that path.
- When the pump coroutine is dropped while suspended, `~DrainingReader` (`src/workerd/api/streams/readable.c++`) releases the reader lock via `ReadableLockImpl::releaseReader(maybeJs = nullptr)`, which only releases the lock — it does not cancel or error the JS source.

As a result, the source's `cancel()` never runs, later `enqueue()` calls do not throw, and the stream runs to natural completion. The `ENABLE_DRAINING_READ_ON_STANDARD_STREAMS` autogate key was removed entirely, so the old behavior is not recoverable with a compatibility date or runtime flag.

## Impact

- Long-running streaming responses keep consuming CPU/wall time after the client disconnects; an orphaned async source runs to natural completion (or, without pending work, stays suspended until isolate teardown).
- Without `enable_request_signal` — which has no default enable date — Worker code has no visible disconnect hook at all for this case.

## Proposed fix

Fix PR: https://github.com/cloudflare/workerd/pull/6833

From `pumpToImpl`'s teardown, when the coroutine is dropped before settling, schedule the source cancel as a waitUntil task so `IncomingRequest::drain()` keeps the context open until the JS `cancel()` runs — the same scheduling `WorkerEntrypoint` uses for its disconnect abort task. The isolate lock is unavailable during coroutine teardown, and the pump promise can itself be owned by the IoContext's task sets (so the drop can happen during `~IoContext`); an `IoContext::WeakRef` guard (the pattern documented at `IoContext::getWeakRef()`) makes that case a safe no-op. A `pumpSettled` flag suppresses the cancel on the normal-completion and error paths, and cancelling with `kj::none` (undefined reason) matches the pre-regression `PumpToReader` drop behavior. Includes regression tests for both the drop-mid-stream and clean-completion cases.

## Tested environment

- OS: Ubuntu 24.04.4 LTS, `x86_64`
- Node.js: `v24.16.0`, npm: `11.17.0`
- workerd npm packages: `1.20260617.1`, `1.20260619.1`, `1.20260717.1`

Contributor guide

Open the contributing guide

Research direction

Read src/workerd/api/streams/standard.c++ and src/workerd/api/streams/readable.c++, focusing on pumpToImpl, DrainingReader teardown, and ReadableLockImpl::releaseReader. Use the linked reproduction to confirm the mid-stream disconnect behavior, then inspect PR 6833 and its regression tests. Done means cancellation occurs after a dropped pump while clean completion remains unaffected.

Written by the indexing model from the issue text.

Assessment

Tech stack
cpp, javascript
Domain
api, backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Clearly specified
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.