http2: fix onread assert when destroying session from stream handler · nodejs/node@9a2f77a · GitHub
Skip to content

Commit 9a2f77a

Browse files
sankalpsthakuraduh95
authored andcommitted
http2: fix onread assert when destroying session from stream handler
When session.destroy() runs from a 'stream' handler, MakeCallback drains nextTick while nghttp2 is still inside mem_recv. Close is deferred for that window (see #64166), so later HEADERS in the same buffer created C++ streams without a JS wrapper or onread, and DATA delivery aborted with Assertion failed: onread->IsFunction(). - Reject new streams while the session is closing - Destroy the C++ handle if on_headers runs after JS destroy - Drop DATA when onread is not installed (defensive) Fixes: #64850 Signed-off-by: Sankalp Thakur <sankalphimself@gmail.com> PR-URL: #65116 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Tim Perry <pimterry@gmail.com>
1 parent b4b8593 commit 9a2f77a

3 files changed

Lines changed: 78 additions & 1 deletion

File tree

‎lib/internal/http2/core.js‎

Lines changed: 8 additions & 1 deletion

‎src/node_http2.cc‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1054,6 +1054,16 @@ int Http2Session::OnBeginHeadersCallback(nghttp2_session* handle,
10541054
// The common case is that we're creating a new stream. The less likely
10551055
// case is that we're receiving a set of trailers
10561056
if (!stream) [[likely]] {
1057+
// Close() may be deferred while mem_recv is in progress (see
1058+
// Http2Session::Close). A 'stream' handler that calls session.destroy()
1059+
// runs via nextTick from MakeCallback during that window, so later
1060+
// HEADERS in the same receive buffer must not create a C++ stream
1061+
// whose JS wrapper (and onread) is never installed.
1062+
if (session->is_closing()) {
1063+
nghttp2_submit_rst_stream(
1064+
session->session(), NGHTTP2_FLAG_NONE, id, NGHTTP2_REFUSED_STREAM);
1065+
return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
1066+
}
10571067
if (!session->CanAddStream() ||
10581068
Http2Stream::New(session, id, frame->headers.cat) == nullptr)
10591069
[[unlikely]] {
Lines changed: 60 additions & 0 deletions

0 commit comments

Comments
 (0)