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

child_process: support AbortSignal in fork #36603

Conversation

benjamingr
Copy link
Member

@benjamingr benjamingr commented Dec 22, 2020

Keeping the AbortSignal support in our APIs up - add the same sort of AbortSignal support we have in .spawn and .execFile to .fork.

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

@benjamingr benjamingr added child_process Issues and PRs related to the child_process subsystem. semver-minor PRs that contain new features and should be released in the next minor version. labels Dec 22, 2020
@benjamingr benjamingr force-pushed the child-process-fork-abortcontroller branch from fafb88e to 6a7fa1a Compare December 22, 2020 13:30
@@ -390,6 +393,7 @@ changes:
done on Windows. Ignored on Unix. **Default:** `false`.
* `uid` {number} Sets the user identity of the process (see setuid(2)).
* `gid` {number} Sets the group identity of the process (see setgid(2)).
* `signal` {AbortSignal} allows closing the subprocess using an AbortSignal.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
* `signal` {AbortSignal} allows closing the subprocess using an AbortSignal.
* `signal` {AbortSignal} Allows closing the subprocess using an AbortSignal.

Copy link
Member

Choose a reason for hiding this comment

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

Optional nit: The options are in alphabetical order up until uid. Move uid, gid, and signal to the locations in the list to make the list fully alphabetical order.

@Trott Trott added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 22, 2020
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Dec 22, 2020
@nodejs-github-bot
Copy link
Collaborator

@benjamingr benjamingr force-pushed the child-process-fork-abortcontroller branch from 6a7fa1a to 18ae659 Compare December 24, 2020 15:13
@benjamingr
Copy link
Member Author

@Trott PTAL (commit queue needs a "fresh" review)

@jasnell @addaleax would love a review - I'm trying to make child_process AbortSignal consistent among different methods.

@benjamingr benjamingr added the request-ci Add this label to start a Jenkins CI on a PR. label Dec 24, 2020
@github-actions github-actions bot 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

@benjamingr benjamingr added request-ci Add this label to start a Jenkins CI on a PR. commit-queue Add this label to land a pull request using GitHub Actions. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Dec 24, 2020
@github-actions github-actions bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Dec 24, 2020
@github-actions
Copy link
Contributor

Landed in 33d99b6...73a21e4

@github-actions github-actions bot closed this Dec 24, 2020
nodejs-github-bot pushed a commit that referenced this pull request Dec 24, 2020
PR-URL: #36603
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
danielleadams pushed a commit that referenced this pull request Jan 12, 2021
PR-URL: #36603
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@danielleadams danielleadams 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)
targos pushed a commit to targos/node that referenced this pull request Apr 24, 2021
PR-URL: nodejs#36603
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit to targos/node that referenced this pull request Apr 26, 2021
PR-URL: nodejs#36603
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit to targos/node that referenced this pull request Apr 30, 2021
PR-URL: nodejs#36603
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
targos pushed a commit that referenced this pull request Apr 30, 2021
PR-URL: #36603
Backport-PR-URL: #38386
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@danielleadams danielleadams mentioned this pull request May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
child_process Issues and PRs related to the child_process subsystem. semver-minor PRs 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.

7 participants