Skip to content

[v26.x backport] http2: fix write deadlock exposed by larger window sizes - #65953

Open
panva wants to merge 2 commits into
nodejs:v26.x-stagingfrom
panva:backport-65440-to-v26.x
Open

[v26.x backport] http2: fix write deadlock exposed by larger window sizes#65953
panva wants to merge 2 commits into
nodejs:v26.x-stagingfrom
panva:backport-65440-to-v26.x

Conversation

@panva

@panva panva commented Sep 10, 2026

Copy link
Copy Markdown
Member

Backports #65440 with a prerequisite partial backport of #63249 to address the reset-cleanup hang seen in #65881.

The #63249 backport only adds peer-reset tracking and allows reset streams to be destroyed after readable 'end' without waiting for pending writes to finish. It preserves 26.x buffered reads, errors, and compat events, without taking the semver-major behavior changes. The #65440 patch is unchanged but present here so that both can be tested together.

Signed-off-by: Tim Perry <pimterry@gmail.com>
PR-URL: nodejs#63249
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Stephen Belanger <admin@stephenbelanger.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Assisted-by: Codex
This removes a guard (no reads while write pending) that creates
this deadlock, which was added as a security mechanism. This guard is
redundant given then other existing mechanisms, and a test is added to
demonstrate that.

Signed-off-by: Tim Perry <pimterry@gmail.com>
PR-URL: nodejs#65440
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Filip Skokan <panva.ip@gmail.com>
Signed-off-by: Filip Skokan <panva.ip@gmail.com>
Assisted-by: Codex
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/http2
  • @nodejs/net
  • @nodejs/performance

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. v26.x Issues that can be reproduced on v26.x or PRs targeting the v26.x-staging branch. labels Sep 10, 2026
@nodejs-github-bot

This comment was marked as outdated.

@mcollina mcollina left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@codecov

codecov Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.65217% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.40%. Comparing base (ca26282) to head (d41cbc0).
⚠️ Report is 421 commits behind head on v26.x-staging.

Files with missing lines Patch % Lines
src/node_http2.cc 94.11% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@                Coverage Diff                @@
##           v26.x-staging   #65953      +/-   ##
=================================================
+ Coverage          90.24%   90.40%   +0.15%     
=================================================
  Files                729      744      +15     
  Lines             242760   250426    +7666     
  Branches           46044    47770    +1726     
=================================================
+ Hits              219073   226389    +7316     
- Misses             15133    15334     +201     
- Partials            8554     8703     +149     
Files with missing lines Coverage Δ
lib/internal/http2/core.js 95.51% <100.00%> (-0.01%) ⬇️
src/node_http2.h 91.90% <100.00%> (+0.04%) ⬆️
src/node_http2.cc 82.10% <94.11%> (+0.23%) ⬆️

... and 162 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. v26.x Issues that can be reproduced on v26.x or PRs targeting the v26.x-staging branch.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants