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

worker: perform initial port.unref() before preload modules #33455

Closed
wants to merge 1 commit into from

Conversation

addaleax
Copy link
Member

The refcount of the internal communication port is relevant for
stdio, but the port.unref() call effectively resets any .ref()
calls happening during stdio operations happening before it.

Therefore, do the .unref() call before loading preload modules,
which may cause stdio operations.

Fixes: #31777

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

The refcount of the internal communication port is relevant for
stdio, but the `port.unref()` call effectively resets any `.ref()`
calls happening during stdio operations happening before it.

Therefore, do the `.unref()` call before loading preload modules,
which may cause stdio operations.

Fixes: nodejs#31777
@addaleax addaleax added the worker Issues and PRs related to Worker support. label May 18, 2020
@nodejs-github-bot
Copy link
Collaborator

@addaleax addaleax added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 19, 2020
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

jasnell pushed a commit that referenced this pull request May 22, 2020
The refcount of the internal communication port is relevant for
stdio, but the `port.unref()` call effectively resets any `.ref()`
calls happening during stdio operations happening before it.

Therefore, do the `.unref()` call before loading preload modules,
which may cause stdio operations.

Fixes: #31777

PR-URL: #33455
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
@jasnell
Copy link
Member

jasnell commented May 22, 2020

Landed in c45313b

@jasnell jasnell closed this May 22, 2020
codebytere pushed a commit that referenced this pull request Jun 18, 2020
The refcount of the internal communication port is relevant for
stdio, but the `port.unref()` call effectively resets any `.ref()`
calls happening during stdio operations happening before it.

Therefore, do the `.unref()` call before loading preload modules,
which may cause stdio operations.

Fixes: #31777

PR-URL: #33455
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
@codebytere codebytere mentioned this pull request Jun 28, 2020
codebytere pushed a commit that referenced this pull request Jul 8, 2020
The refcount of the internal communication port is relevant for
stdio, but the `port.unref()` call effectively resets any `.ref()`
calls happening during stdio operations happening before it.

Therefore, do the `.unref()` call before loading preload modules,
which may cause stdio operations.

Fixes: #31777

PR-URL: #33455
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
@codebytere codebytere mentioned this pull request Jul 13, 2020
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. worker Issues and PRs related to Worker support.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

workers: worker doesn't write all data to stdout stream with NODE_OPTIONS --require
6 participants