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.

Sign up for GitHub

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

Jump to bottom

[v10.x backport] backport 23233, 22938 #23398

Closed
wants to merge 2 commits into from
Closed

[v10.x backport] backport 23233, 22938 #23398

wants to merge 2 commits into from

Conversation

ofrobots
Copy link
Contributor

@ofrobots ofrobots commented Oct 10, 2018 •
edited
Loading

@nodejs-github-bot
Copy link
Collaborator

@ofrobots build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1189/pipeline

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. v10.x labels Oct 10, 2018
@ofrobots ofrobots mentioned this pull request Oct 10, 2018
Closed
2 tasks
@ofrobots ofrobots changed the title [v10.x backport] src: ready background workers before bootstrap [v10.x backport] backport 23233, 22938 Oct 10, 2018
@ofrobots
Copy link
Contributor Author

To reduce blocking, I added the back port of #22938 to this PR as well.

@ofrobots ofrobots mentioned this pull request Oct 10, 2018
Closed
2 tasks
@richardlau richardlau mentioned this pull request Oct 10, 2018
Merged
@ofrobots
src: ready background workers before bootstrap
63f0e77 
Make sure background workers are ready before proceeding with the
bootstrap or post-bootstrap execution of any code that may trigger
`process.exit()`.

Fixes: nodejs#23065

PR-URL: nodejs#23233
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
@ofrobots
trace_events: destroy platform before tracing
afca579 
For safer shutdown, we should destroy the platform – and background
threads - before the tracing infrastructure is destroyed. This change
fixes the relative order of NodePlatform disposition and the tracing
agent shutting down. This matches the nesting order for startup.

Make the tracing agent own the tracing controller instead of platform
to match the above.

Fixes: nodejs#22865

PR-URL: nodejs#22938
Reviewed-By: Eugene Ostroukhov <eostroukhov@google.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@Trott
Copy link
Member

Trott commented Oct 13, 2018

CI is green.

@Trott Trott added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 13, 2018
@ofrobots
Copy link
Contributor Author

Landed as aecba73..5fea320.

@ofrobots ofrobots closed this Oct 15, 2018
@ofrobots ofrobots deleted the backport-10-23233 branch October 15, 2018 19:40
@targos
Copy link
Member

targos commented Oct 16, 2018

@ofrobots not a big deal, but commits from backport PR should have a Backport-PR-URL added in the metadata.

@ofrobots
Copy link
Contributor Author

Sorry about that. I'll keep it in mind for the next one.

@ofrobots ofrobots mentioned this pull request Oct 16, 2018
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@targos targos targos approved these changes

@thefourtheye thefourtheye thefourtheye approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@ofrobots @nodejs-github-bot @Trott @targos @thefourtheye