Skip to content

src: move per-process global variables into node::per_process #25302

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

Closed
joyeecheung wants to merge 2 commits into from
Closed

src: move per-process global variables into node::per_process #25302

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
11ed69e
src: move per-process global variables into node::per_process
joyeecheung Jan 1, 2019
3a58828
fixup! src: move per-process global variables into node::per_process
joyeecheung Jan 4, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
fixup! src: move per-process global variables into node::per_process
  • Loading branch information
@joyeecheung
joyeecheung committed Jan 4, 2019
commit 3a58828bd49aaf3185ab885a65b3f3d889356aef
5 changes: 4 additions & 1 deletion src/node.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,12 @@ using v8::Value;

namespace per_process {
// Tells whether --prof is passed.
// TODO(joyeecheung): replace this with env->options()->prof_process
// TODO(joyeecheung): move env->options()->prof_process to
// per_process::cli_options.prof_process and use that instead.
static bool v8_is_profiling = false;

// TODO(joyeecheung): these are no longer necessary. Remove them.
// See: https://github.com/nodejs/node/pull/25302#discussion_r244924196
// Isolate on the main thread
static Mutex main_isolate_mutex;
static Isolate* main_isolate;
Copy link
Copy Markdown
Member

@addaleax addaleax Jan 1, 2019

Choose a reason for hiding this comment

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

Are these actually consumed anywhere? It looks like we should remove this entirely…

Copy link
Copy Markdown
Member Author

@joyeecheung joyeecheung Jan 1, 2019

Choose a reason for hiding this comment

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

I think they are used to check that someone don't call Start multiple times and accidentally dispose the isolate being used in another thread..though I am having a hard time imaging how that would be possible with the current code...

Copy link
Copy Markdown
Member

@bnoordhuis bnoordhuis Jan 3, 2019

Choose a reason for hiding this comment

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

node_isolate is kind of vestigial at this point, see commit 75adde0 from 2014. We couldn't remove it outright back then but we can now.

The mutex was introduced to fix a SIGUSR1 race, see commit 844f0a9. It too can be removed.

Expand Down