Skip to content

Commit

Permalink
src: remove uses of node::InitializeV8Platform()
Browse files Browse the repository at this point in the history
This requires minor changes to src/env.cc to deal with
`node::tracing::AgentWriterHandle::GetTracingController()` now possibly
returning a nullptr, because the cctest doesn't set one.

It seems plausible to me that embedders won't set one either so that
seems like an okay change to make. It avoids embedders having to track
down nullptr segfaults.

PR-URL: #31245
Refs: #31217
Reviewed-By: David Carlier <devnexen@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
  • Loading branch information
bnoordhuis authored and addaleax committed Jan 9, 2020
1 parent aff6fff commit 93b0e39
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 8 deletions.
8 changes: 4 additions & 4 deletions src/env.cc
Expand Up @@ -332,8 +332,8 @@ Environment::Environment(IsolateData* isolate_data,

if (tracing::AgentWriterHandle* writer = GetTracingAgentWriter()) {
trace_state_observer_ = std::make_unique<TrackingTraceStateObserver>(this);
TracingController* tracing_controller = writer->GetTracingController();
tracing_controller->AddTraceStateObserver(trace_state_observer_.get());
if (TracingController* tracing_controller = writer->GetTracingController())
tracing_controller->AddTraceStateObserver(trace_state_observer_.get());
}

destroy_async_id_list_.reserve(512);
Expand Down Expand Up @@ -409,8 +409,8 @@ Environment::~Environment() {
if (trace_state_observer_) {
tracing::AgentWriterHandle* writer = GetTracingAgentWriter();
CHECK_NOT_NULL(writer);
TracingController* tracing_controller = writer->GetTracingController();
tracing_controller->RemoveTraceStateObserver(trace_state_observer_.get());
if (TracingController* tracing_controller = writer->GetTracingController())
tracing_controller->RemoveTraceStateObserver(trace_state_observer_.get());
}

delete[] http_parser_buffer_;
Expand Down
3 changes: 2 additions & 1 deletion src/node.cc
Expand Up @@ -1077,7 +1077,8 @@ InitializationResult InitializeOncePerProcess(int argc, char** argv) {
V8::SetEntropySource(crypto::EntropySource);
#endif // HAVE_OPENSSL

InitializeV8Platform(per_process::cli_options->v8_thread_pool_size);
per_process::v8_platform.Initialize(
per_process::cli_options->v8_thread_pool_size);
V8::Initialize();
performance::performance_v8_start = PERFORMANCE_NOW();
per_process::v8_initialized = true;
Expand Down
10 changes: 7 additions & 3 deletions test/cctest/node_test_fixture.h
Expand Up @@ -80,9 +80,13 @@ class NodeZeroIsolateTestFixture : public ::testing::Test {

tracing_agent = std::make_unique<node::tracing::Agent>();
node::tracing::TraceEventHelper::SetAgent(tracing_agent.get());
node::tracing::TracingController* tracing_controller =
tracing_agent->GetTracingController();
CHECK_EQ(0, uv_loop_init(&current_loop));
platform.reset(static_cast<node::NodePlatform*>(
node::InitializeV8Platform(4)));
static constexpr int kV8ThreadPoolSize = 4;
platform.reset(
new node::NodePlatform(kV8ThreadPoolSize, tracing_controller));
v8::V8::InitializePlatform(platform.get());
v8::V8::Initialize();
}

Expand All @@ -108,7 +112,7 @@ class NodeTestFixture : public NodeZeroIsolateTestFixture {

void SetUp() override {
NodeZeroIsolateTestFixture::SetUp();
isolate_ = NewIsolate(allocator.get(), &current_loop);
isolate_ = NewIsolate(allocator.get(), &current_loop, platform.get());
CHECK_NOT_NULL(isolate_);
isolate_->Enter();
}
Expand Down

0 comments on commit 93b0e39

Please sign in to comment.