Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Commit a8daccc

Browse filesBrowse files
joyeecheungaddaleax
authored andcommitted
src: move per-process global variables into node::per_process
So that it's easier to tell whether we are manipulating per-process global states that may need to be treated with care to avoid races. Also added comments about these variables and moved some of them to a more suitable compilation unit: - Move `v8_initialized` to `util.h` since it's only used in `util.cc` and `node.cc` - Rename `process_mutex` to `tty_mutex` and move it into `node_errors.cc` since that's the only place it's used to guard the tty. - Move `per_process_opts_mutex` and `per_process_opts` into `node_options.h` and rename them to `per_process::cli_options[_mutex]` - Rename `node_isolate[_mutex]` to `per_process::main_isolate[_mutex]` PR-URL: #25302 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
1 parent 582c34d commit a8daccc
Copy full SHA for a8daccc
Expand file treeCollapse file tree

17 files changed

+126
-92
lines changed
Open diff view settings
Collapse file

‎src/env.cc‎

Copy file name to clipboardExpand all lines: src/env.cc
+2-1Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,8 @@ IsolateData::IsolateData(Isolate* isolate,
7979
if (platform_ != nullptr)
8080
platform_->RegisterIsolate(isolate_, event_loop);
8181

82-
options_.reset(new PerIsolateOptions(*per_process_opts->per_isolate));
82+
options_.reset(
83+
new PerIsolateOptions(*(per_process::cli_options->per_isolate)));
8384

8485
// Create string and private symbol properties as internalized one byte
8586
// strings after the platform is properly initialized.
Collapse file

‎src/node.cc‎

Copy file name to clipboardExpand all lines: src/node.cc
+51-39Lines changed: 51 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -139,21 +139,31 @@ using v8::Undefined;
139139
using v8::V8;
140140
using v8::Value;
141141

142+
namespace per_process {
143+
// Tells whether --prof is passed.
144+
// TODO(joyeecheung): move env->options()->prof_process to
145+
// per_process::cli_options.prof_process and use that instead.
142146
static bool v8_is_profiling = false;
143147

144-
// Bit flag used to track security reverts (see node_revert.h)
145-
unsigned int reverted = 0;
148+
// TODO(joyeecheung): these are no longer necessary. Remove them.
149+
// See: https://github.com/nodejs/node/pull/25302#discussion_r244924196
150+
// Isolate on the main thread
151+
static Mutex main_isolate_mutex;
152+
static Isolate* main_isolate;
146153

154+
// node_revert.h
155+
// Bit flag used to track security reverts.
156+
unsigned int reverted_cve = 0;
157+
158+
// util.h
159+
// Tells whether the per-process V8::Initialize() is called and
160+
// if it is safe to call v8::Isolate::GetCurrent().
147161
bool v8_initialized = false;
148162

163+
// node_internals.h
149164
// process-relative uptime base, initialized at start-up
150165
double prog_start_time;
151-
152-
Mutex per_process_opts_mutex;
153-
std::shared_ptr<PerProcessOptions> per_process_opts {
154-
new PerProcessOptions() };
155-
static Mutex node_isolate_mutex;
156-
static Isolate* node_isolate;
166+
} // namespace per_process
157167

158168
// Ensures that __metadata trace events are only emitted
159169
// when tracing is enabled.
@@ -269,14 +279,15 @@ static struct {
269279
#endif // HAVE_INSPECTOR
270280

271281
void StartTracingAgent() {
272-
if (per_process_opts->trace_event_categories.empty()) {
282+
if (per_process::cli_options->trace_event_categories.empty()) {
273283
tracing_file_writer_ = tracing_agent_->DefaultHandle();
274284
} else {
275285
tracing_file_writer_ = tracing_agent_->AddClient(
276-
ParseCommaSeparatedSet(per_process_opts->trace_event_categories),
286+
ParseCommaSeparatedSet(
287+
per_process::cli_options->trace_event_categories),
277288
std::unique_ptr<tracing::AsyncTraceWriter>(
278289
new tracing::NodeTraceWriter(
279-
per_process_opts->trace_event_file_pattern)),
290+
per_process::cli_options->trace_event_file_pattern)),
280291
tracing::Agent::kUseDefaultCategories);
281292
}
282293
}
@@ -504,7 +515,7 @@ const char* signo_string(int signo) {
504515
}
505516

506517
void* ArrayBufferAllocator::Allocate(size_t size) {
507-
if (zero_fill_field_ || per_process_opts->zero_fill_all_buffers)
518+
if (zero_fill_field_ || per_process::cli_options->zero_fill_all_buffers)
508519
return UncheckedCalloc(size);
509520
else
510521
return UncheckedMalloc(size);
@@ -1254,12 +1265,12 @@ void ProcessArgv(std::vector<std::string>* args,
12541265
{
12551266
// TODO(addaleax): The mutex here should ideally be held during the
12561267
// entire function, but that doesn't play well with the exit() calls below.
1257-
Mutex::ScopedLock lock(per_process_opts_mutex);
1268+
Mutex::ScopedLock lock(per_process::cli_options_mutex);
12581269
options_parser::PerProcessOptionsParser::instance.Parse(
12591270
args,
12601271
exec_args,
12611272
&v8_args,
1262-
per_process_opts.get(),
1273+
per_process::cli_options.get(),
12631274
is_env ? kAllowedInEnvironment : kDisallowedInEnvironment,
12641275
&errors);
12651276
}
@@ -1271,20 +1282,20 @@ void ProcessArgv(std::vector<std::string>* args,
12711282
exit(9);
12721283
}
12731284

1274-
if (per_process_opts->print_version) {
1285+
if (per_process::cli_options->print_version) {
12751286
printf("%s\n", NODE_VERSION);
12761287
exit(0);
12771288
}
12781289

1279-
if (per_process_opts->print_v8_help) {
1290+
if (per_process::cli_options->print_v8_help) {
12801291
V8::SetFlagsFromString("--help", 6);
12811292
exit(0);
12821293
}
12831294

1284-
for (const std::string& cve : per_process_opts->security_reverts)
1295+
for (const std::string& cve : per_process::cli_options->security_reverts)
12851296
Revert(cve.c_str());
12861297

1287-
auto env_opts = per_process_opts->per_isolate->per_env;
1298+
auto env_opts = per_process::cli_options->per_isolate->per_env;
12881299
if (std::find(v8_args.begin(), v8_args.end(),
12891300
"--abort-on-uncaught-exception") != v8_args.end() ||
12901301
std::find(v8_args.begin(), v8_args.end(),
@@ -1297,14 +1308,14 @@ void ProcessArgv(std::vector<std::string>* args,
12971308
// behavior but it could also interfere with the user's intentions in ways
12981309
// we fail to anticipate. Dillema.
12991310
if (std::find(v8_args.begin(), v8_args.end(), "--prof") != v8_args.end()) {
1300-
v8_is_profiling = true;
1311+
per_process::v8_is_profiling = true;
13011312
}
13021313

13031314
#ifdef __POSIX__
13041315
// Block SIGPROF signals when sleeping in epoll_wait/kevent/etc. Avoids the
13051316
// performance penalty of frequent EINTR wakeups when the profiler is running.
13061317
// Only do this for v8.log profiling, as it breaks v8::CpuProfiler users.
1307-
if (v8_is_profiling) {
1318+
if (per_process::v8_is_profiling) {
13081319
uv_loop_configure(uv_default_loop(), UV_LOOP_BLOCK_SIGNAL, SIGPROF);
13091320
}
13101321
#endif
@@ -1333,7 +1344,7 @@ void ProcessArgv(std::vector<std::string>* args,
13331344
void Init(std::vector<std::string>* argv,
13341345
std::vector<std::string>* exec_argv) {
13351346
// Initialize prog_start_time to get relative uptime.
1336-
prog_start_time = static_cast<double>(uv_now(uv_default_loop()));
1347+
per_process::prog_start_time = static_cast<double>(uv_now(uv_default_loop()));
13371348

13381349
// Register built-in modules
13391350
binding::RegisterBuiltinModules();
@@ -1349,7 +1360,7 @@ void Init(std::vector<std::string>* argv,
13491360
#endif
13501361

13511362
std::shared_ptr<EnvironmentOptions> default_env_options =
1352-
per_process_opts->per_isolate->per_env;
1363+
per_process::cli_options->per_isolate->per_env;
13531364
{
13541365
std::string text;
13551366
default_env_options->pending_deprecation =
@@ -1378,7 +1389,7 @@ void Init(std::vector<std::string>* argv,
13781389
}
13791390

13801391
#if HAVE_OPENSSL
1381-
std::string* openssl_config = &per_process_opts->openssl_config;
1392+
std::string* openssl_config = &per_process::cli_options->openssl_config;
13821393
if (openssl_config->empty()) {
13831394
credentials::SafeGetenv("OPENSSL_CONF", openssl_config);
13841395
}
@@ -1412,16 +1423,17 @@ void Init(std::vector<std::string>* argv,
14121423
ProcessArgv(argv, exec_argv, false);
14131424

14141425
// Set the process.title immediately after processing argv if --title is set.
1415-
if (!per_process_opts->title.empty())
1416-
uv_set_process_title(per_process_opts->title.c_str());
1426+
if (!per_process::cli_options->title.empty())
1427+
uv_set_process_title(per_process::cli_options->title.c_str());
14171428

14181429
#if defined(NODE_HAVE_I18N_SUPPORT)
14191430
// If the parameter isn't given, use the env variable.
1420-
if (per_process_opts->icu_data_dir.empty())
1421-
credentials::SafeGetenv("NODE_ICU_DATA", &per_process_opts->icu_data_dir);
1431+
if (per_process::cli_options->icu_data_dir.empty())
1432+
credentials::SafeGetenv("NODE_ICU_DATA",
1433+
&per_process::cli_options->icu_data_dir);
14221434
// Initialize ICU.
14231435
// If icu_data_dir is empty here, it will load the 'minimal' data.
1424-
if (!i18n::InitializeICUDirectory(per_process_opts->icu_data_dir)) {
1436+
if (!i18n::InitializeICUDirectory(per_process::cli_options->icu_data_dir)) {
14251437
fprintf(stderr,
14261438
"%s: could not initialize ICU "
14271439
"(check NODE_ICU_DATA or --icu-data-dir parameters)\n",
@@ -1582,7 +1594,7 @@ Environment* CreateEnvironment(IsolateData* isolate_data,
15821594
std::vector<std::string> args(argv, argv + argc);
15831595
std::vector<std::string> exec_args(exec_argv, exec_argv + exec_argc);
15841596
Environment* env = new Environment(isolate_data, context);
1585-
env->Start(args, exec_args, v8_is_profiling);
1597+
env->Start(args, exec_args, per_process::v8_is_profiling);
15861598
return env;
15871599
}
15881600

@@ -1656,7 +1668,7 @@ inline int Start(Isolate* isolate, IsolateData* isolate_data,
16561668
Local<Context> context = NewContext(isolate);
16571669
Context::Scope context_scope(context);
16581670
Environment env(isolate_data, context);
1659-
env.Start(args, exec_args, v8_is_profiling);
1671+
env.Start(args, exec_args, per_process::v8_is_profiling);
16601672

16611673
const char* path = args.size() > 1 ? args[1].c_str() : nullptr;
16621674
StartInspector(&env, path);
@@ -1763,9 +1775,9 @@ inline int Start(uv_loop_t* event_loop,
17631775
return 12; // Signal internal error.
17641776

17651777
{
1766-
Mutex::ScopedLock scoped_lock(node_isolate_mutex);
1767-
CHECK_NULL(node_isolate);
1768-
node_isolate = isolate;
1778+
Mutex::ScopedLock scoped_lock(per_process::main_isolate_mutex);
1779+
CHECK_NULL(per_process::main_isolate);
1780+
per_process::main_isolate = isolate;
17691781
}
17701782

17711783
int exit_code;
@@ -1790,9 +1802,9 @@ inline int Start(uv_loop_t* event_loop,
17901802
}
17911803

17921804
{
1793-
Mutex::ScopedLock scoped_lock(node_isolate_mutex);
1794-
CHECK_EQ(node_isolate, isolate);
1795-
node_isolate = nullptr;
1805+
Mutex::ScopedLock scoped_lock(per_process::main_isolate_mutex);
1806+
CHECK_EQ(per_process::main_isolate, isolate);
1807+
per_process::main_isolate = nullptr;
17961808
}
17971809

17981810
isolate->Dispose();
@@ -1840,14 +1852,14 @@ int Start(int argc, char** argv) {
18401852
V8::SetEntropySource(crypto::EntropySource);
18411853
#endif // HAVE_OPENSSL
18421854

1843-
InitializeV8Platform(per_process_opts->v8_thread_pool_size);
1855+
InitializeV8Platform(per_process::cli_options->v8_thread_pool_size);
18441856
V8::Initialize();
18451857
performance::performance_v8_start = PERFORMANCE_NOW();
1846-
v8_initialized = true;
1858+
per_process::v8_initialized = true;
18471859
const int exit_code =
18481860
Start(uv_default_loop(), args, exec_args);
18491861
v8_platform.StopTracingAgent();
1850-
v8_initialized = false;
1862+
per_process::v8_initialized = false;
18511863
V8::Dispose();
18521864

18531865
// uv_run cannot be called from the time before the beforeExit callback
Collapse file

‎src/node_buffer.cc‎

Copy file name to clipboardExpand all lines: src/node_buffer.cc
+3-3Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -59,9 +59,9 @@ namespace node {
5959
namespace {
6060

6161
inline void* BufferMalloc(size_t length) {
62-
return per_process_opts->zero_fill_all_buffers ?
63-
node::UncheckedCalloc(length) :
64-
node::UncheckedMalloc(length);
62+
return per_process::cli_options->zero_fill_all_buffers ?
63+
node::UncheckedCalloc(length) :
64+
node::UncheckedMalloc(length);
6565
}
6666

6767
} // namespace
Collapse file

‎src/node_config.cc‎

Copy file name to clipboardExpand all lines: src/node_config.cc
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ static void Initialize(Local<Object> target,
4141
#ifdef NODE_FIPS_MODE
4242
READONLY_TRUE_PROPERTY(target, "fipsMode");
4343
// TODO(addaleax): Use options parser variable instead.
44-
if (per_process_opts->force_fips_crypto)
44+
if (per_process::cli_options->force_fips_crypto)
4545
READONLY_TRUE_PROPERTY(target, "fipsForced");
4646
#endif
4747

@@ -69,7 +69,7 @@ static void Initialize(Local<Object> target,
6969

7070
// TODO(addaleax): This seems to be an unused, private API. Remove it?
7171
READONLY_STRING_PROPERTY(target, "icuDataDir",
72-
per_process_opts->icu_data_dir);
72+
per_process::cli_options->icu_data_dir);
7373

7474
#endif // NODE_HAVE_I18N_SUPPORT
7575

Collapse file

‎src/node_constants.cc‎

Copy file name to clipboardExpand all lines: src/node_constants.cc
+4-3Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1234,9 +1234,10 @@ void DefineCryptoConstants(Local<Object> target) {
12341234
NODE_DEFINE_STRING_CONSTANT(target,
12351235
"defaultCoreCipherList",
12361236
DEFAULT_CIPHER_LIST_CORE);
1237-
NODE_DEFINE_STRING_CONSTANT(target,
1238-
"defaultCipherList",
1239-
per_process_opts->tls_cipher_list.c_str());
1237+
NODE_DEFINE_STRING_CONSTANT(
1238+
target,
1239+
"defaultCipherList",
1240+
per_process::cli_options->tls_cipher_list.c_str());
12401241

12411242
NODE_DEFINE_CONSTANT(target, TLS1_VERSION);
12421243
NODE_DEFINE_CONSTANT(target, TLS1_1_VERSION);
Collapse file

‎src/node_credentials.cc‎

Copy file name to clipboardExpand all lines: src/node_credentials.cc
+1-1Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,7 @@ bool SafeGetenv(const char* key, std::string* text) {
3838
#endif
3939

4040
{
41-
Mutex::ScopedLock lock(environ_mutex);
41+
Mutex::ScopedLock lock(per_process::env_var_mutex);
4242
if (const char* value = getenv(key)) {
4343
*text = value;
4444
return true;
Collapse file

‎src/node_crypto.cc‎

Copy file name to clipboardExpand all lines: src/node_crypto.cc
+8-9Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -769,7 +769,7 @@ static X509_STORE* NewRootCertStore() {
769769
if (*system_cert_path != '\0') {
770770
X509_STORE_load_locations(store, system_cert_path, nullptr);
771771
}
772-
if (per_process_opts->ssl_openssl_cert_store) {
772+
if (per_process::cli_options->ssl_openssl_cert_store) {
773773
X509_STORE_set_default_paths(store);
774774
} else {
775775
for (X509* cert : root_certs_vector) {
@@ -6153,16 +6153,15 @@ void InitCryptoOnce() {
61536153
OPENSSL_no_config();
61546154

61556155
// --openssl-config=...
6156-
if (!per_process_opts->openssl_config.empty()) {
6156+
if (!per_process::cli_options->openssl_config.empty()) {
61576157
OPENSSL_load_builtin_modules();
61586158
#ifndef OPENSSL_NO_ENGINE
61596159
ENGINE_load_builtin_engines();
61606160
#endif
61616161
ERR_clear_error();
6162-
CONF_modules_load_file(
6163-
per_process_opts->openssl_config.c_str(),
6164-
nullptr,
6165-
CONF_MFLAGS_DEFAULT_SECTION);
6162+
CONF_modules_load_file(per_process::cli_options->openssl_config.c_str(),
6163+
nullptr,
6164+
CONF_MFLAGS_DEFAULT_SECTION);
61666165
int err = ERR_get_error();
61676166
if (0 != err) {
61686167
fprintf(stderr,
@@ -6178,8 +6177,8 @@ void InitCryptoOnce() {
61786177
#ifdef NODE_FIPS_MODE
61796178
/* Override FIPS settings in cnf file, if needed. */
61806179
unsigned long err = 0; // NOLINT(runtime/int)
6181-
if (per_process_opts->enable_fips_crypto ||
6182-
per_process_opts->force_fips_crypto) {
6180+
if (per_process::cli_options->enable_fips_crypto ||
6181+
per_process::cli_options->force_fips_crypto) {
61836182
if (0 == FIPS_mode() && !FIPS_mode_set(1)) {
61846183
err = ERR_get_error();
61856184
}
@@ -6242,7 +6241,7 @@ void GetFipsCrypto(const FunctionCallbackInfo<Value>& args) {
62426241
}
62436242

62446243
void SetFipsCrypto(const FunctionCallbackInfo<Value>& args) {
6245-
CHECK(!per_process_opts->force_fips_crypto);
6244+
CHECK(!per_process::cli_options->force_fips_crypto);
62466245
Environment* env = Environment::GetCurrent(args);
62476246
const bool enabled = FIPS_mode();
62486247
bool enable;

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.