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 timeout with null handle #16489

Closed

Conversation

apapirovski
Copy link
Member

@apapirovski apapirovski commented Oct 25, 2017

Seems like there's a scenario where _onTimeout somehow gets called with _handle set to null. If anyone has any ideas on how this is possible then let me know so I can put together a better test than I have so far.

This might need to be expedited (< 48 hrs) and turned into 8.8.1? Not sure... 😔 😓 At least it's not LTS yet... *sigh*

Fixes: #16484
Refs: #15791

/cc @MylesBorins

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • commit message follows commit guidelines
Affected core subsystem(s)

net, test

@apapirovski apapirovski added confirmed-bug Issues with confirmed bugs. net Issues and PRs related to the net subsystem. labels Oct 25, 2017
@nodejs-github-bot nodejs-github-bot added the net Issues and PRs related to the net subsystem. label Oct 25, 2017
@apapirovski apapirovski removed the confirmed-bug Issues with confirmed bugs. label Oct 25, 2017
@apapirovski
Copy link
Member Author

@apapirovski apapirovski added this to the 9.0.0 milestone Oct 25, 2017
@apapirovski
Copy link
Member Author

apapirovski commented Oct 25, 2017

CI seems good other than one persistently failing ARM machine (unrelated).

Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

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

Yeah, I’d be good with pushing out 8.8.1 asap – after all, that was a security release.

@nodejs/release

Copy link
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

LGTM, +1 for fast tracking a patch release as long as it's not done on friday :-)

@cjihrig
Copy link
Contributor

cjihrig commented Oct 25, 2017

Landed in cecbb59. Thanks!

@cjihrig cjihrig closed this Oct 25, 2017
cjihrig pushed a commit that referenced this pull request Oct 25, 2017
This commit handles the case where _onTimeout is called with a
null handle.

Refs: #15791
Fixes: #16484
PR-URL: #16489
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
addaleax added a commit to addaleax/node-core-utils that referenced this pull request Oct 25, 2017
@apapirovski apapirovski deleted the patch-fix-net-timeout-throw branch October 25, 2017 19:09
cjihrig pushed a commit that referenced this pull request Oct 25, 2017
This commit handles the case where _onTimeout is called with a
null handle.

Refs: #15791
Fixes: #16484
PR-URL: #16489
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
cjihrig added a commit that referenced this pull request Oct 25, 2017
Notable changes:

* net:
  - Fix timeout with null handle issue. This is a regression in
    Node 8.8.0. #16489
@cjihrig cjihrig mentioned this pull request Oct 25, 2017
apapirovski added a commit to apapirovski/node that referenced this pull request Oct 25, 2017
This commit handles the case where _onTimeout is called with a
null handle.

Refs: nodejs#15791
Fixes: nodejs#16484
PR-URL: nodejs#16489
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
cjihrig added a commit that referenced this pull request Oct 25, 2017
Notable changes:

* net:
  - Fix timeout with null handle issue. This is a regression in
    Node 8.8.0. #16489
addaleax added a commit to addaleax/node-core-utils that referenced this pull request Oct 26, 2017
joyeecheung pushed a commit to nodejs/node-core-utils that referenced this pull request Oct 26, 2017
addaleax pushed a commit to ayojs/ayo that referenced this pull request Oct 26, 2017
This commit handles the case where _onTimeout is called with a
null handle.

Refs: nodejs/node#15791
Fixes: nodejs/node#16484
PR-URL: nodejs/node#16489
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Oct 26, 2017
Notable changes:

* net:
  - Fix timeout with null handle issue. This is a regression in
    Node 8.8.0. nodejs/node#16489
@MylesBorins
Copy link
Contributor

Should this be backported to v6.x-staging? If yes please follow the guide and raise a backport PR, if not let me know or add the dont-land-on label.

addaleax pushed a commit to ayojs/ayo that referenced this pull request Dec 7, 2017
This commit handles the case where _onTimeout is called with a
null handle.

Refs: nodejs/node#15791
Fixes: nodejs/node#16484
PR-URL: nodejs/node#16489
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
addaleax pushed a commit to ayojs/ayo that referenced this pull request Dec 7, 2017
Notable changes:

* net:
  - Fix timeout with null handle issue. This is a regression in
    Node 8.8.0. nodejs/node#16489
apapirovski added a commit to apapirovski/node that referenced this pull request Dec 12, 2017
This commit handles the case where _onTimeout is called with a
null handle.

Refs: nodejs#15791
Fixes: nodejs#16484
PR-URL: nodejs#16489
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
MylesBorins pushed a commit that referenced this pull request Dec 20, 2017
This commit handles the case where _onTimeout is called with a
null handle.

Backport-PR-URL: #16420
Refs: #15791
Fixes: #16484
PR-URL: #16489
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
MylesBorins pushed a commit that referenced this pull request Jan 2, 2018
This commit handles the case where _onTimeout is called with a
null handle.

Backport-PR-URL: #16420
Refs: #15791
Fixes: #16484
PR-URL: #16489
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
@MylesBorins MylesBorins mentioned this pull request Jan 24, 2018
johnfrench3 pushed a commit to johnfrench3/core-utils-node that referenced this pull request Nov 2, 2022
renawolford6 added a commit to renawolford6/node-dev-build-core-utils that referenced this pull request Nov 10, 2022
Developerarif2 pushed a commit to Developerarif2/node-core-utils that referenced this pull request Jan 27, 2023
gerkai added a commit to gerkai/node-core-utils-project-build that referenced this pull request Jan 27, 2023
shovon58 pushed a commit to shovon58/node-core-utils that referenced this pull request Jun 9, 2023
patrickm68 added a commit to patrickm68/NodeJS-core-utils that referenced this pull request Sep 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
net Issues and PRs related to the net subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

net.js broken in node 8.8
7 participants