GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — net: support AbortSignal in server.listen by Linkgoron · Pull Request #36623 · nodejs/node

Conversation

@Linkgoron

@Linkgoron Linkgoron commented Dec 24, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

net: support AbortSignal in server.listen

Add AbortSignal support to server.listen

  • 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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@nodejs-github-bot nodejs-github-bot added the net Issues and PRs related to the net subsystem. label Dec 24, 2020
@Linkgoron Linkgoron changed the title net: support abortSignal in server.listen net: support AbortSignal in server.listen Dec 24, 2020
benjamingr
benjamingr reviewed Dec 24, 2020
View reviewed changes
Comment thread doc/api/net.md Outdated

@benjamingr benjamingr Dec 24, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

a docs example would be nice - namely mentioning this calls .close internally and doesn't stop ongoing connections the server is already serving.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@Linkgoron Linkgoron Dec 27, 2020

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

added

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@benjamingr benjamingr added request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. semver-minor PRs that contain new features and should be released in the next minor version. labels Dec 24, 2020
benjamingr
benjamingr approved these changes Dec 24, 2020
View reviewed changes

@benjamingr benjamingr left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Thanks 🙏

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Dec 24, 2020
@nodejs-github-bot

nodejs-github-bot commented Dec 24, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/35067/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
aduh95
aduh95 reviewed Dec 26, 2020
View reviewed changes
Comment thread lib/net.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread lib/net.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
Comment thread lib/net.js Outdated

@aduh95 aduh95 Dec 26, 2020

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

If this function is only used once, is it relevant to have it as a separate function instead of inlining it?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@Linkgoron Linkgoron Dec 27, 2020

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

I just thought that it would make the listen function harder to read.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
benjamingr reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
Comment thread lib/internal/streams/add-abort-signal.js Outdated

@aduh95 aduh95 Dec 26, 2020

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Unrelated change, but good catch ;) it might be worth having it land in a separate PR (in case this one was reverted or didn't get backported to LTS release lines).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread lib/net.js Resolved

Uh oh!

There was an error while loading. Please reload this page.
@benjamingr benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Dec 27, 2020
Nitzan Uziely
net: support abortSignal in server.listen
8bfc031
@Linkgoron
Linkgoron force-pushed the net-server-listen-abortcontroller branch from f7e8509 to 8bfc031 Compare December 27, 2020 17:48
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Dec 27, 2020
@nodejs-github-bot

nodejs-github-bot commented Dec 27, 2020

Copy link
Copy Markdown
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/35096/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@benjamingr benjamingr added author ready PRs with CI started, the required approvals, and no outstanding review comments. commit-queue PRs queued for automated landing through the Commit Queue. labels Dec 27, 2020
benjamingr
benjamingr approved these changes Dec 31, 2020
View reviewed changes
@github-actions github-actions Bot added commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. and removed commit-queue PRs queued for automated landing through the Commit Queue. labels Dec 31, 2020
@github-actions

github-actions Bot commented Dec 31, 2020

Copy link
Copy Markdown
Contributor
Commit Queue failed
- Loading data for nodejs/node/pull/36623
✔  Done loading data for nodejs/node/pull/36623
----------------------------------- PR info ------------------------------------
Title      net: support AbortSignal in server.listen (#36623)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     Linkgoron:net-server-listen-abortcontroller -> nodejs:master
Labels     author ready, net, semver-minor
Commits    1
 - net: support abortSignal in server.listen
Committers 1
 - Nitzan Uziely 
PR-URL: https://github.com/nodejs/node/pull/36623
Reviewed-By: Benjamin Gruenbaum 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/36623
Reviewed-By: Benjamin Gruenbaum 
--------------------------------------------------------------------------------
   ✔  Last GitHub Actions successful
   ℹ  Last Full PR CI on 2020-12-27T18:13:03Z: https://ci.nodejs.org/job/node-test-pull-request/35096/
- Querying data for job/node-test-pull-request/35096/
✔  Build data downloaded
   ✔  Last Jenkins CI successful
   ℹ  This PR was created on Thu, 24 Dec 2020 23:02:30 GMT
   ✔  Approvals: 1
   ✔  - Benjamin Gruenbaum (@benjamingr): https://github.com/nodejs/node/pull/36623#pullrequestreview-560508048
   ✖  This PR needs to wait 6 more hours to land (or 0 hours if there is one more approval)
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu
https://github.com/nodejs/node/actions/runs/454778704
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@benjamingr benjamingr removed the commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. label Dec 31, 2020
jasnell
jasnell approved these changes Dec 31, 2020
View reviewed changes
@jasnell

jasnell commented Dec 31, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Landed in 51b4367
benjamingr and Linkgoron reacted with hooray emoji
All reactions
  • 🎉 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@jasnell jasnell closed this Dec 31, 2020
jasnell pushed a commit that referenced this pull request Dec 31, 2020
Nitzan Uziely @jasnell
net: support abortSignal in server.listen
51b4367 
PR-URL: #36623
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
danielleadams pushed a commit that referenced this pull request Jan 12, 2021
Nitzan Uziely @danielleadams
net: support abortSignal in server.listen
02dbcc4 
PR-URL: #36623
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: James M Snell <[email protected]>
@danielleadams danielleadams mentioned this pull request Jan 12, 2021
v15.6.0 proposal #36889
Merged
danielleadams added a commit that referenced this pull request Jan 12, 2021
@danielleadams
2020-01-12, Version 15.6.0 (Current)
7994977 
PR-URL: #36889

Notable changes:

* child_process:
  * add 'overlapped' stdio flag (Thiago Padilha) [#29412](#29412)
  * support AbortSignal in fork (Benjamin Gruenbaum) [#36603](#36603)
* crypto:
  * implement basic secure heap support (James M Snell) [#36779](#36779)
  * fixup bug in keygen error handling (James M Snell) [#36779](#36779)
  * introduce X509Certificate API (James M Snell) [#36804](#36804)
  * implement randomuuid (James M Snell) [#36729](#36729)
* doc:
  * update release key for Danielle Adams (Danielle Adams) [#36793](#36793)
  * add dnlup to collaborators (Daniele Belardi) [#36849](#36849)
  * add panva to collaborators (Filip Skokan) [#36802](#36802)
  * add yashLadha to collaborator (Yash Ladha) [#36666](#36666)
* http:
  * set lifo as the default scheduling strategy in Agent (Matteo Collina) [#36685](#36685)
* net:
  * support abortSignal in server.listen (Nitzan Uziely) [#36623](#36623)
* process:
  * add direct access to rss without iterating pages (Adrien Maret) [#34291](#34291)
* v8:
  * fix native  constructors (ExE Boss) [#36549](#36549)
danielleadams added a commit that referenced this pull request Jan 12, 2021
@danielleadams
2020-01-12, Version 15.6.0 (Current)
ce42dea 
PR-URL: #36889

Notable changes:

* child_process:
  * add 'overlapped' stdio flag (Thiago Padilha) [#29412](#29412)
  * support AbortSignal in fork (Benjamin Gruenbaum) [#36603](#36603)
* crypto:
  * implement basic secure heap support (James M Snell) [#36779](#36779)
  * fixup bug in keygen error handling (James M Snell) [#36779](#36779)
  * introduce X509Certificate API (James M Snell) [#36804](#36804)
  * implement randomuuid (James M Snell) [#36729](#36729)
* doc:
  * update release key for Danielle Adams (Danielle Adams) [#36793](#36793)
  * add dnlup to collaborators (Daniele Belardi) [#36849](#36849)
  * add panva to collaborators (Filip Skokan) [#36802](#36802)
  * add yashLadha to collaborator (Yash Ladha) [#36666](#36666)
* http:
  * set lifo as the default scheduling strategy in Agent (Matteo Collina) [#36685](#36685)
* net:
  * support abortSignal in server.listen (Nitzan Uziely) [#36623](#36623)
* process:
  * add direct access to rss without iterating pages (Adrien Maret) [#34291](#34291)
* v8:
  * fix native  constructors (ExE Boss) [#36549](#36549)
danielleadams added a commit that referenced this pull request Jan 13, 2021
@danielleadams
2020-01-12, Version 15.6.0 (Current)
3d4c36b 
PR-URL: #36889

Notable changes:

* child_process:
  * add 'overlapped' stdio flag (Thiago Padilha) [#29412](#29412)
  * support AbortSignal in fork (Benjamin Gruenbaum) [#36603](#36603)
* crypto:
  * implement basic secure heap support (James M Snell) [#36779](#36779)
  * fixup bug in keygen error handling (James M Snell) [#36779](#36779)
  * introduce X509Certificate API (James M Snell) [#36804](#36804)
  * implement randomuuid (James M Snell) [#36729](#36729)
* doc:
  * update release key for Danielle Adams (Danielle Adams) [#36793](#36793)
  * add dnlup to collaborators (Daniele Belardi) [#36849](#36849)
  * add panva to collaborators (Filip Skokan) [#36802](#36802)
  * add yashLadha to collaborator (Yash Ladha) [#36666](#36666)
* http:
  * set lifo as the default scheduling strategy in Agent (Matteo Collina) [#36685](#36685)
* net:
  * support abortSignal in server.listen (Nitzan Uziely) [#36623](#36623)
* process:
  * add direct access to rss without iterating pages (Adrien Maret) [#34291](#34291)
* v8:
  * fix native  constructors (ExE Boss) [#36549](#36549)
danielleadams added a commit that referenced this pull request Jan 13, 2021
@danielleadams
2020-01-13, Version 15.6.0 (Current)
cf44a24 
PR-URL: #36889

Notable changes:

* child_process:
  * add 'overlapped' stdio flag (Thiago Padilha) [#29412](#29412)
  * support AbortSignal in fork (Benjamin Gruenbaum) [#36603](#36603)
* crypto:
  * implement basic secure heap support (James M Snell) [#36779](#36779)
  * fixup bug in keygen error handling (James M Snell) [#36779](#36779)
  * introduce X509Certificate API (James M Snell) [#36804](#36804)
  * implement randomuuid (James M Snell) [#36729](#36729)
* doc:
  * update release key for Danielle Adams (Danielle Adams) [#36793](#36793)
  * add dnlup to collaborators (Daniele Belardi) [#36849](#36849)
  * add panva to collaborators (Filip Skokan) [#36802](#36802)
  * add yashLadha to collaborator (Yash Ladha) [#36666](#36666)
* http:
  * set lifo as the default scheduling strategy in Agent (Matteo Collina) [#36685](#36685)
* net:
  * support abortSignal in server.listen (Nitzan Uziely) [#36623](#36623)
* process:
  * add direct access to rss without iterating pages (Adrien Maret) [#34291](#34291)
* v8:
  * fix native  constructors (ExE Boss) [#36549](#36549)
danielleadams added a commit that referenced this pull request Jan 13, 2021
@danielleadams
2021-01-13, Version 15.6.0 (Current)
1a5dbf3 
PR-URL: #36889

Notable changes:

* child_process:
  * add 'overlapped' stdio flag (Thiago Padilha) [#29412](#29412)
  * support AbortSignal in fork (Benjamin Gruenbaum) [#36603](#36603)
* crypto:
  * implement basic secure heap support (James M Snell) [#36779](#36779)
  * fixup bug in keygen error handling (James M Snell) [#36779](#36779)
  * introduce X509Certificate API (James M Snell) [#36804](#36804)
  * implement randomuuid (James M Snell) [#36729](#36729)
* doc:
  * update release key for Danielle Adams (Danielle Adams) [#36793](#36793)
  * add dnlup to collaborators (Daniele Belardi) [#36849](#36849)
  * add panva to collaborators (Filip Skokan) [#36802](#36802)
  * add yashLadha to collaborator (Yash Ladha) [#36666](#36666)
* http:
  * set lifo as the default scheduling strategy in Agent (Matteo Collina) [#36685](#36685)
* net:
  * support abortSignal in server.listen (Nitzan Uziely) [#36623](#36623)
* process:
  * add direct access to rss without iterating pages (Adrien Maret) [#34291](#34291)
* v8:
  * fix native  constructors (ExE Boss) [#36549](#36549)
danielleadams added a commit that referenced this pull request Jan 14, 2021
@danielleadams
2021-01-14, Version 15.6.0 (Current)
4c6744c 
PR-URL: #36889

Notable changes:

* child_process:
  * add 'overlapped' stdio flag (Thiago Padilha) [#29412](#29412)
  * support AbortSignal in fork (Benjamin Gruenbaum) [#36603](#36603)
* crypto:
  * implement basic secure heap support (James M Snell) [#36779](#36779)
  * fixup bug in keygen error handling (James M Snell) [#36779](#36779)
  * introduce X509Certificate API (James M Snell) [#36804](#36804)
  * implement randomuuid (James M Snell) [#36729](#36729)
* doc:
  * update release key for Danielle Adams (Danielle Adams) [#36793](#36793)
  * add dnlup to collaborators (Daniele Belardi) [#36849](#36849)
  * add panva to collaborators (Filip Skokan) [#36802](#36802)
  * add yashLadha to collaborator (Yash Ladha) [#36666](#36666)
* http:
  * set lifo as the default scheduling strategy in Agent (Matteo Collina) [#36685](#36685)
* net:
  * support abortSignal in server.listen (Nitzan Uziely) [#36623](#36623)
* process:
  * add direct access to rss without iterating pages (Adrien Maret) [#34291](#34291)
* v8:
  * fix native  constructors (ExE Boss) [#36549](#36549)
danielleadams added a commit that referenced this pull request Jan 14, 2021
@danielleadams
2021-01-14, Version 15.6.0 (Current)
1700bbd 
PR-URL: #36889

Notable changes:

* child_process:
  * add 'overlapped' stdio flag (Thiago Padilha) (#29412)
  * support AbortSignal in fork (Benjamin Gruenbaum) (#36603)
* crypto:
  * implement basic secure heap support (James M Snell) (#36779)
  * fixup bug in keygen error handling (James M Snell) (#36779)
  * introduce X509Certificate API (James M Snell) (#36804)
  * implement randomuuid (James M Snell) (#36729)
* doc:
  * update release key for Danielle Adams (Danielle Adams) (#36793)
  * add dnlup to collaborators (Daniele Belardi) (#36849)
  * add panva to collaborators (Filip Skokan) (#36802)
  * add yashLadha to collaborator (Yash Ladha) (#36666)
* http:
  * set lifo as the default scheduling strategy in Agent (Matteo Collina) (#36685)
* net:
  * support abortSignal in server.listen (Nitzan Uziely) (#36623)
* process:
  * add direct access to rss without iterating pages (Adrien Maret) (#34291)
* v8:
  * fix native  constructors (ExE Boss) (#36549)
danielleadams added a commit that referenced this pull request Jan 15, 2021
@danielleadams
2021-01-14, Version 15.6.0 (Current)
0f78948 
PR-URL: #36889

Notable changes:

* child_process:
  * add 'overlapped' stdio flag (Thiago Padilha) (#29412)
  * support AbortSignal in fork (Benjamin Gruenbaum) (#36603)
* crypto:
  * implement basic secure heap support (James M Snell) (#36779)
  * fixup bug in keygen error handling (James M Snell) (#36779)
  * introduce X509Certificate API (James M Snell) (#36804)
  * implement randomuuid (James M Snell) (#36729)
* doc:
  * update release key for Danielle Adams (Danielle Adams) (#36793)
  * add dnlup to collaborators (Daniele Belardi) (#36849)
  * add panva to collaborators (Filip Skokan) (#36802)
  * add yashLadha to collaborator (Yash Ladha) (#36666)
* http:
  * set lifo as the default scheduling strategy in Agent (Matteo Collina) (#36685)
* net:
  * support abortSignal in server.listen (Nitzan Uziely) (#36623)
* process:
  * add direct access to rss without iterating pages (Adrien Maret) (#34291)
* v8:
  * fix native  constructors (ExE Boss) (#36549)
@Linkgoron
Linkgoron deleted the net-server-listen-abortcontroller branch January 22, 2021 22:08
@targos targos added dont-land-on-v12.x labels Aug 8, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aduh95 aduh95 aduh95 left review comments
@jasnell jasnell jasnell approved these changes
@benjamingr benjamingr benjamingr approved these changes

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. net Issues and PRs related to the net subsystem. semver-minor PRs that contain new features and should be released in the next minor version.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

6 participants

@Linkgoron @nodejs-github-bot @jasnell @benjamingr @aduh95 @targos
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.