Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/runtime/webcore/fetch/FetchTasklet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1622,6 +1622,12 @@
if this.signal_store.body_receive_mode() == BodyReceiveMode::Ignore {
return;
}
// reader.cancel() on a still-arriving body aborts the fetch so the
// connection closes and the server observes the cancellation. A fully
// received body is left to drain/cleanup so the socket stays reusable.
if this.result.has_more {
this.abort_task();
}

Check failure on line 1630 in src/runtime/webcore/fetch/FetchTasklet.rs

View check run for this annotation

Claude / Claude Code Review

Unsynchronized read of result.has_more in on_stream_cancelled_callback

This reads `this.result.has_more` on the JS thread without holding `this.mutex`, but the HTTP thread's `callback()` wholesale-replaces `task_ref.result` under that mutex (line 2356) — every other JS-thread reader of `self.result` (e.g. `on_progress_update` at line 809, the sibling stream-start callback at line 1578) locks first. Since `reader.cancel()` fires precisely while the body is still arriving, the HTTP thread can be mid-move of `result`, so this is a data race; a stale `has_more == false
Comment thread
robobun marked this conversation as resolved.
Outdated
this.ignore_remaining_response_body(false);
}

Expand Down Expand Up @@ -1772,8 +1778,14 @@
{
self.schedule_receive_resume();
}
let aborted = self.signal_store.aborted.load(Ordering::Relaxed);
if let Some(http_) = self.http.as_mut() {
http_.enable_response_body_streaming();
// An aborted fetch (reader.cancel() on an in-flight body, or an
// AbortSignal) is already shutting the connection down; draining
// would read the rest of an unbounded body and hold the socket open.
if !aborted {
http_.enable_response_body_streaming();
}
Comment thread
robobun marked this conversation as resolved.
}
// we should not keep the process alive if we are ignoring the body
let _ = self.javascript_vm;
Expand Down
45 changes: 45 additions & 0 deletions test/js/web/fetch/fetch.stream.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1428,4 +1428,49 @@ describe.concurrent("fetch() with streaming", () => {
expect(new TextDecoder().decode(result.value!)).toBe("hello\n");
server.kill("SIGTERM");
});

it("reader.cancel() aborts the request and triggers the server's stream cancel (#33227)", async () => {
const { promise: sawAbort, resolve: onAbort } = Promise.withResolvers<void>();
const { promise: sawCancel, resolve: onCancel } = Promise.withResolvers<void>();
const { promise: holdOpen } = Promise.withResolvers<void>();
const encoder = new TextEncoder();

using server = Bun.serve({
port: 0,
fetch(request) {
request.signal.addEventListener("abort", () => onAbort());
let count = 0;
return new Response(
new ReadableStream({
async pull(controller) {
if (count < 8) {
controller.enqueue(encoder.encode(`data: ${count++}\n\n`));
} else {
// Keep the body in-flight (never closes) so the client is
// cancelling a response that is still arriving.
await holdOpen;
}
},
cancel() {
onCancel();
},
}),
{ headers: { "Content-Type": "text/event-stream" } },
);
},
});

const res = await fetch(`http://localhost:${server.port}`, { method: "POST" });
const reader = res.body!.getReader();

const first = await reader.read();
expect(first.done).toBe(false);
expect(first.value!.byteLength).toBeGreaterThan(0);

await reader.cancel();

// Cancelling the reader must abort the fetch and close the connection, so
// the server observes request.signal's abort and runs the body stream's cancel().
await Promise.all([sawAbort, sawCancel]);
});
});
Loading