Skip to content

net: Fix invalid write after end error - #36043

Closed
ronag wants to merge 8 commits into
nodejs:masterfrom
nxtedition:net-write-after-end-no-half-open
Closed

net: Fix invalid write after end error#36043
ronag wants to merge 8 commits into
nodejs:masterfrom
nxtedition:net-write-after-end-no-half-open

Conversation

@ronag

@ronagronag commented Nov 8, 2020

Copy link
Copy Markdown
Member

Don't error if not ended.

Fixes: #36029

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

@ronagronag added the net Issues and PRs related to the net subsystem. label Nov 8, 2020
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/net
  • @nodejs/quic

Don't error if not ended.
Fixes: nodejs#36029
@ronag
ronagforce-pushed the net-write-after-end-no-half-open branch from 4c50922 to 1f73c8aCompareNovember 8, 2020 18:58
Comment threadlib/net.js Outdated
Comment threadtest/parallel/test-net-writable.js Outdated
Comment threadtest/parallel/test-net-writable.js Outdated
ronagand others added 2 commits November 8, 2020 23:27
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>

@mcollinamcollina 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


Should this be backported to v14 or v12?

@mscdex

Copy link
Copy Markdown
Contributor

@mcollina Only if efefdd6 ever gets backported to those branches

@mcollina

Copy link
Copy Markdown
Member

that's semver-major, so no.

@mcollina

Copy link
Copy Markdown
Member

Thanks @mscdex!

@Trott

Trott commented Nov 9, 2020

Copy link
Copy Markdown
Member

The test-stream-transform-end.js test file added here is timing out in GitHub CI.

Comment threadtest/parallel/test-net-writable.js
@ronagronag added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Nov 9, 2020
Comment threadtest/parallel/test-stream-transform-end.js Outdated
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 9, 2020
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@ronag
ronagforce-pushed the net-write-after-end-no-half-open branch from 61d67ad to a0ea1b0CompareNovember 9, 2020 22:52
@ronagronag added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 10, 2020
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 10, 2020
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@ronagronag added the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 10, 2020
@github-actionsgithub-actionsBot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 10, 2020
@github-actions

Copy link
Copy Markdown
Contributor
Commit Queue failed
- Loading data for nodejs/node/pull/36043
✔ Done loading data for nodejs/node/pull/36043
----------------------------------- PR info ------------------------------------
Title net: Fix invalid write after end error (#36043)
Author Robert Nagy (@ronag)
Branch ronag:net-write-after-end-no-half-open -> nodejs:master
Labels author ready, dont-land-on-v10.x, dont-land-on-v12.x, dont-land-on-v14.x, net
Commits 8
- net: Fix invalid write after end error
- fixup
- Update lib/net.js
- fixup
- fixup
- fixup
- fixup
- fixup
Committers 2
- Robert Nagy - GitHub PR-URL: https://github.com/nodejs/node/pull/36043
Fixes: https://github.com/nodejs/node/issues/36029
Reviewed-By: Matteo Collina Reviewed-By: Luigi Pinca ------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/36043
Fixes: https://github.com/nodejs/node/issues/36029
Reviewed-By: Matteo Collina Reviewed-By: Luigi Pinca --------------------------------------------------------------------------------
⚠ Commits were pushed since the last review:
⚠ - fixup
✔ Last GitHub Actions successful
ℹ Last Full PR CI on 2020-11-10T10:34:15Z: https://ci.nodejs.org/job/node-test-pull-request/34279/
- Querying data for job/node-test-pull-request/34279/
✔ Build data downloaded
✔ Last Jenkins CI successful
ℹ This PR was created on Sun, 08 Nov 2020 18:57:57 GMT
✔ Approvals: 2
✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/36043#pullrequestreview-525942020
✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/36043#pullrequestreview-526614630
✖ This PR needs to wait 1 more hours to land
--------------------------------------------------------------------------------
✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu

Commit Queue action: https://github.com/nodejs/node/actions/runs/356263944

@github-actionsgithub-actionsBot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Nov 10, 2020
@aduh95aduh95 added commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Nov 10, 2020
@github-actionsgithub-actionsBot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 10, 2020
@github-actions

Copy link
Copy Markdown
Contributor
Commit Queue failed
- Loading data for nodejs/node/pull/36043
✔ Done loading data for nodejs/node/pull/36043
----------------------------------- PR info ------------------------------------
Title net: Fix invalid write after end error (#36043)
Author Robert Nagy (@ronag)
Branch ronag:net-write-after-end-no-half-open -> nodejs:master
Labels author ready, commit-queue, dont-land-on-v10.x, dont-land-on-v12.x, dont-land-on-v14.x, net
Commits 8
- net: Fix invalid write after end error
- fixup
- Update lib/net.js
- fixup
- fixup
- fixup
- fixup
- fixup
Committers 2
- Robert Nagy - GitHub PR-URL: https://github.com/nodejs/node/pull/36043
Fixes: https://github.com/nodejs/node/issues/36029
Reviewed-By: Matteo Collina Reviewed-By: Luigi Pinca ------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/36043
Fixes: https://github.com/nodejs/node/issues/36029
Reviewed-By: Matteo Collina Reviewed-By: Luigi Pinca --------------------------------------------------------------------------------
⚠ Commits were pushed since the last review:
⚠ - fixup
✔ Last GitHub Actions successful
ℹ Last Full PR CI on 2020-11-10T17:01:28Z: https://ci.nodejs.org/job/node-test-pull-request/34279/
- Querying data for job/node-test-pull-request/34279/
✔ Build data downloaded
✔ Last Jenkins CI successful
ℹ This PR was created on Sun, 08 Nov 2020 18:57:57 GMT
✔ Approvals: 2
✔ - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/36043#pullrequestreview-525942020
✔ - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/36043#pullrequestreview-526614630
--------------------------------------------------------------------------------
✔ Aborted `git node land` session in /home/runner/work/node/node/.ncu

Commit Queue action: https://github.com/nodejs/node/actions/runs/356556511

@github-actionsgithub-actionsBot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Nov 10, 2020
@aduh95

Copy link
Copy Markdown
Contributor

Landed in f7f0a6a

@aduh95aduh95 closed this Nov 10, 2020
aduh95 pushed a commit that referenced this pull request Nov 10, 2020
Don't error if not ended.
Fixes: #36029
PR-URL: #36043
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
codebytere pushed a commit that referenced this pull request Nov 22, 2020
Don't error if not ended.
Fixes: #36029
PR-URL: #36043
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@codebyterecodebytere mentioned this pull request Nov 22, 2020
@targostargos removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Sep 5, 2021
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author readyPRs that have at least one approval, no pending requests for changes, and a CI started.netIssues and PRs related to the net subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

net: writable is not set to false after socket has ended

8 participants

@ronag@nodejs-github-bot@mscdex@mcollina@Trott@aduh95@lpinca@targos