Skip to content

Commit

Permalink
src: add public APIs to manage v8::TracingController
Browse files Browse the repository at this point in the history
We added a hack for this a while ago for Electron, so let’s remove
that hack and make this an official API.

Refs: nodejs#28724
Refs: nodejs#33800

PR-URL: nodejs#33850
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
  • Loading branch information
addaleax committed Jun 15, 2020
1 parent f645cc7 commit b371213
Show file tree
Hide file tree
Showing 7 changed files with 43 additions and 6 deletions.
9 changes: 9 additions & 0 deletions src/node.h
Expand Up @@ -505,6 +505,15 @@ NODE_EXTERN MultiIsolatePlatform* CreatePlatform(
v8::TracingController* tracing_controller);
NODE_EXTERN void FreePlatform(MultiIsolatePlatform* platform);

// Get/set the currently active tracing controller. Using CreatePlatform()
// will implicitly set this by default. This is global and should be initialized
// along with the v8::Platform instance that is being used. `controller`
// is allowed to be `nullptr`.
// This is used for tracing events from Node.js itself. V8 uses the tracing
// controller returned from the active `v8::Platform` instance.
NODE_EXTERN v8::TracingController* GetTracingController();
NODE_EXTERN void SetTracingController(v8::TracingController* controller);

NODE_EXTERN void EmitBeforeExit(Environment* env);
NODE_EXTERN int EmitExit(Environment* env);
NODE_EXTERN void RunAtExit(Environment* env);
Expand Down
3 changes: 2 additions & 1 deletion src/node_platform.cc
Expand Up @@ -333,7 +333,8 @@ NodePlatform::NodePlatform(int thread_pool_size,
// TODO(addaleax): It's a bit icky that we use global state here, but we can't
// really do anything about it unless V8 starts exposing a way to access the
// current v8::Platform instance.
tracing::TraceEventHelper::SetTracingController(tracing_controller_);
SetTracingController(tracing_controller_);
DCHECK_EQ(GetTracingController(), tracing_controller_);
worker_thread_task_runner_ =
std::make_shared<WorkerThreadsTaskRunner>(thread_pool_size);
}
Expand Down
1 change: 0 additions & 1 deletion src/node_platform.h
Expand Up @@ -11,7 +11,6 @@
#include "libplatform/libplatform.h"
#include "node.h"
#include "node_mutex.h"
#include "tracing/agent.h"
#include "uv.h"

namespace node {
Expand Down
1 change: 1 addition & 0 deletions src/node_v8_platform-inl.h
Expand Up @@ -8,6 +8,7 @@
#include "env-inl.h"
#include "node.h"
#include "node_metadata.h"
#include "node_platform.h"
#include "node_options.h"
#include "tracing/node_trace_writer.h"
#include "tracing/trace_event.h"
Expand Down
10 changes: 10 additions & 0 deletions src/tracing/trace_event.cc
@@ -1,4 +1,5 @@
#include "tracing/trace_event.h"
#include "node.h"

namespace node {
namespace tracing {
Expand All @@ -24,4 +25,13 @@ void TraceEventHelper::SetTracingController(v8::TracingController* controller) {
}

} // namespace tracing

v8::TracingController* GetTracingController() {
return tracing::TraceEventHelper::GetTracingController();
}

void SetTracingController(v8::TracingController* controller) {
tracing::TraceEventHelper::SetTracingController(controller);
}

} // namespace node
6 changes: 2 additions & 4 deletions src/tracing/trace_event.h
Expand Up @@ -5,8 +5,8 @@
#ifndef SRC_TRACING_TRACE_EVENT_H_
#define SRC_TRACING_TRACE_EVENT_H_

#include "node_platform.h"
#include "v8-platform.h"
#include "tracing/agent.h"
#include "trace_event_common.h"
#include <atomic>

Expand Down Expand Up @@ -310,9 +310,7 @@ const int kZeroNumArgs = 0;
const decltype(nullptr) kGlobalScope = nullptr;
const uint64_t kNoId = 0;

// Extern (for now) because embedders need access to TraceEventHelper.
// Refs: https://github.com/nodejs/node/pull/28724
class NODE_EXTERN TraceEventHelper {
class TraceEventHelper {
public:
static v8::TracingController* GetTracingController();
static void SetTracingController(v8::TracingController* controller);
Expand Down
19 changes: 19 additions & 0 deletions test/cctest/test_platform.cc
Expand Up @@ -104,3 +104,22 @@ TEST_F(NodeZeroIsolateTestFixture, IsolatePlatformDelegateTest) {
platform->UnregisterIsolate(isolate);
isolate->Dispose();
}

TEST_F(PlatformTest, TracingControllerNullptr) {
v8::TracingController* orig_controller = node::GetTracingController();
node::SetTracingController(nullptr);
EXPECT_EQ(node::GetTracingController(), nullptr);

v8::Isolate::Scope isolate_scope(isolate_);
const v8::HandleScope handle_scope(isolate_);
const Argv argv;
Env env {handle_scope, argv};

node::LoadEnvironment(*env, [&](const node::StartExecutionCallbackInfo& info)
-> v8::MaybeLocal<v8::Value> {
return v8::Null(isolate_);
});

node::SetTracingController(orig_controller);
EXPECT_EQ(node::GetTracingController(), orig_controller);
}

0 comments on commit b371213

Please sign in to comment.