Skip to content

Conversation

@Linkgoron
Copy link
Contributor

@LinkgoronLinkgoron commented Dec 24, 2020

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

@nodejs-github-botnodejs-github-bot added the net Issues and PRs related to the net subsystem. label Dec 24, 2020
@LinkgoronLinkgoron changed the title net: support abortSignal in server.listennet: support AbortSignal in server.listenDec 24, 2020
doc/api/net.md Outdated
Copy link
Member

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.

Copy link
ContributorAuthor

Choose a reason for hiding this comment

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

added

@benjamingrbenjamingr added request-ci Add this label to start a Jenkins CI on a PR. semver-minor PRs that contain new features and should be released in the next minor version. labels Dec 24, 2020
Copy link
Member

@benjamingrbenjamingr left a comment

Choose a reason for hiding this comment

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

Thanks 🙏

@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 24, 2020
@nodejs-github-bot
Copy link
Collaborator

lib/net.js Outdated
Copy link
Contributor

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?

Copy link
ContributorAuthor

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.

Copy link
Contributor

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).

@benjamingrbenjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 27, 2020
@LinkgoronLinkgoronforce-pushed the net-server-listen-abortcontroller branch from f7e8509 to 8bfc031CompareDecember 27, 2020 17:48
@github-actionsgithub-actionsbot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 27, 2020
@nodejs-github-bot
Copy link
Collaborator

@benjamingrbenjamingr added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Dec 27, 2020
@github-actionsgithub-actionsbot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Dec 31, 2020
@github-actions
Copy link
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

@benjamingrbenjamingr removed the commit-queue-failed An error occurred while landing this pull request using GitHub Actions. label Dec 31, 2020
@jasnell
Copy link
Member

jasnell commented Dec 31, 2020

Landed in 51b4367

@jasnelljasnell closed this Dec 31, 2020
jasnell pushed a commit that referenced this pull request Dec 31, 2020
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
PR-URL: #36623 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]>
@danielleadamsdanielleadams mentioned this pull request Jan 12, 2021
danielleadams added a commit that referenced this pull request Jan 12, 2021
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
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
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
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
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
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
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
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)
@LinkgoronLinkgoron deleted the net-server-listen-abortcontroller branch January 22, 2021 22:08
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.semver-minorPRs that contain new features and should be released in the next minor version.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

@Linkgoron@nodejs-github-bot@jasnell@benjamingr@aduh95@targos