Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net: Fix invalid write after end error #36043

Closed

Conversation

ronag
Copy link
Member

@ronag ronag commented Nov 8, 2020

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

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

Review requested:

  • @nodejs/net
  • @nodejs/quic

Don't error if not ended.

Fixes: nodejs#36029
@ronag ronag force-pushed the net-write-after-end-no-half-open branch from 4c50922 to 1f73c8a Compare November 8, 2020 18:58
lib/net.js Outdated Show resolved Hide resolved
ronag and others added 2 commits November 8, 2020 23:27
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
Copy link
Member

@mcollina mcollina left a comment

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
Contributor

mscdex commented Nov 9, 2020

@mcollina Only if efefdd6 ever gets backported to those branches

@mcollina
Copy link
Member

mcollina commented Nov 9, 2020

that's semver-major, so no.

@mcollina
Copy link
Member

mcollina commented Nov 9, 2020

Thanks @mscdex!

@Trott
Copy link
Member

Trott commented Nov 9, 2020

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

@ronag ronag 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
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 9, 2020
@nodejs-github-bot
Copy link
Collaborator

@ronag ronag force-pushed the net-write-after-end-no-half-open branch from 61d67ad to a0ea1b0 Compare November 9, 2020 22:52
@ronag ronag added the request-ci Add this label to start a Jenkins CI on a PR. label Nov 10, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 10, 2020
@nodejs-github-bot
Copy link
Collaborator

@ronag ronag added the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 10, 2020
@github-actions github-actions bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 10, 2020
@github-actions
Copy link
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-actions github-actions bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Nov 10, 2020
@aduh95 aduh95 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-actions github-actions bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 10, 2020
@github-actions
Copy link
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-actions github-actions bot added the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Nov 10, 2020
@aduh95
Copy link
Contributor

aduh95 commented Nov 10, 2020

Landed in f7f0a6a

@aduh95 aduh95 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>
@codebytere codebytere mentioned this pull request Nov 22, 2020
@targos targos removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Sep 5, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. net Issues 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