fix(ext/http): don't panic recycling cancelled record on native response#36053
Merged
Merged
Conversation
When an HTTP/2 request is cancelled while its handler is still running and the handler then returns a native fast-path response (e.g. `new Response()`), Deno could panic in `HttpRecord::recycle` with either "Expected to be last strong reference" or a `RefCell` borrow error. `recycle()` pools the record allocation and requires sole ownership of the `Rc<HttpRecord>`. Every correct caller of `complete()` moves the last reference in, but `HttpRecordExternal::complete` took `&self` and had to clone the `Rc`, leaving the enum's own reference alive. On the cancelled (`been_dropped`) path `complete()` recycles the record while that extra reference is still outstanding, breaking the invariant. Make `HttpRecordExternal::complete` consume `self` so the record is moved into `HttpRecord::complete` instead of cloned. All call sites were already last-use. Closes #36046
bartlomieju
force-pushed
the
fix/http2-recycle-cancelled-native-response
branch
from
July 15, 2026 10:03
9b96179 to
61fdf92
Compare
bartlomieju
added a commit
that referenced
this pull request
Jul 15, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #36046, a panic in
HttpRecord::recyclereported on Windows overHTTP/2. It triggers when a client cancels a request while its handler is
still running and the handler then returns a native fast-path response
(such as
new Response()). Depending on timing it surfaced as eitherHTTP state error: Expected to be last strong referenceor aRefCellborrow error at
ext/http/service.rs.recycle()pools the record's allocation and takes the inner value, so itrequires sole ownership of the
Rc<HttpRecord>. Every correct caller ofcomplete()moves the last reference in (for exampleset_responseownshttp: Rc<HttpRecord>and callshttp.complete()). The native-responsepath added in #34446 routes through the
HttpRecordExternalenum, whosecompletemethod took&selfand therefore had to clone theRc. Thatclone left the enum's own reference alive, so on the cancelled
(
been_dropped) pathcomplete()reachedfinishandrecyclewhile anextra reference was still outstanding, violating the invariant.
The fix makes
HttpRecordExternal::completeconsumeself, moving therecord into
HttpRecord::completerather than cloning it. All call siteswere already using the value for the last time, so no other changes were
needed.
The added unit test reproduces the crash: it cancels HTTP/2 streams (over
TLS, which is where the hyper record path is used) while the handler is
still pending, then confirms the server stays healthy. It panics the
pre-fix binary and passes after the change.