Skip to content

Commit 5362fef

Browse files
addaleaxcodebytere
authored andcommitted
src: add public APIs to manage v8::TracingController
We added a hack for this a while ago for Electron, so let’s remove that hack and make this an official API. Refs: #28724 Refs: #33800 PR-URL: #33850 Reviewed-By: Michaël Zasso <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Michael Dawson <[email protected]>
1 parent 01d8b91 commit 5362fef

7 files changed

+43
-6
lines changed

src/node.h

+9
Original file line numberDiff line numberDiff line change
@@ -510,6 +510,15 @@ NODE_EXTERN MultiIsolatePlatform* CreatePlatform(
510510
v8::TracingController* tracing_controller);
511511
NODE_EXTERN void FreePlatform(MultiIsolatePlatform* platform);
512512

513+
// Get/set the currently active tracing controller. Using CreatePlatform()
514+
// will implicitly set this by default. This is global and should be initialized
515+
// along with the v8::Platform instance that is being used. `controller`
516+
// is allowed to be `nullptr`.
517+
// This is used for tracing events from Node.js itself. V8 uses the tracing
518+
// controller returned from the active `v8::Platform` instance.
519+
NODE_EXTERN v8::TracingController* GetTracingController();
520+
NODE_EXTERN void SetTracingController(v8::TracingController* controller);
521+
513522
NODE_EXTERN void EmitBeforeExit(Environment* env);
514523
NODE_EXTERN int EmitExit(Environment* env);
515524
NODE_EXTERN void RunAtExit(Environment* env);

src/node_platform.cc

+2-1
Original file line numberDiff line numberDiff line change
@@ -333,7 +333,8 @@ NodePlatform::NodePlatform(int thread_pool_size,
333333
// TODO(addaleax): It's a bit icky that we use global state here, but we can't
334334
// really do anything about it unless V8 starts exposing a way to access the
335335
// current v8::Platform instance.
336-
tracing::TraceEventHelper::SetTracingController(tracing_controller_);
336+
SetTracingController(tracing_controller_);
337+
DCHECK_EQ(GetTracingController(), tracing_controller_);
337338
worker_thread_task_runner_ =
338339
std::make_shared<WorkerThreadsTaskRunner>(thread_pool_size);
339340
}

src/node_platform.h

-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@
1111
#include "libplatform/libplatform.h"
1212
#include "node.h"
1313
#include "node_mutex.h"
14-
#include "tracing/agent.h"
1514
#include "uv.h"
1615

1716
namespace node {

src/node_v8_platform-inl.h

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include "env-inl.h"
99
#include "node.h"
1010
#include "node_metadata.h"
11+
#include "node_platform.h"
1112
#include "node_options.h"
1213
#include "tracing/node_trace_writer.h"
1314
#include "tracing/trace_event.h"

src/tracing/trace_event.cc

+10
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
#include "tracing/trace_event.h"
2+
#include "node.h"
23

34
namespace node {
45
namespace tracing {
@@ -24,4 +25,13 @@ void TraceEventHelper::SetTracingController(v8::TracingController* controller) {
2425
}
2526

2627
} // namespace tracing
28+
29+
v8::TracingController* GetTracingController() {
30+
return tracing::TraceEventHelper::GetTracingController();
31+
}
32+
33+
void SetTracingController(v8::TracingController* controller) {
34+
tracing::TraceEventHelper::SetTracingController(controller);
35+
}
36+
2737
} // namespace node

src/tracing/trace_event.h

+2-4
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,8 @@
55
#ifndef SRC_TRACING_TRACE_EVENT_H_
66
#define SRC_TRACING_TRACE_EVENT_H_
77

8-
#include "node_platform.h"
98
#include "v8-platform.h"
9+
#include "tracing/agent.h"
1010
#include "trace_event_common.h"
1111
#include <atomic>
1212

@@ -310,9 +310,7 @@ const int kZeroNumArgs = 0;
310310
const decltype(nullptr) kGlobalScope = nullptr;
311311
const uint64_t kNoId = 0;
312312

313-
// Extern (for now) because embedders need access to TraceEventHelper.
314-
// Refs: https://github.com/nodejs/node/pull/28724
315-
class NODE_EXTERN TraceEventHelper {
313+
class TraceEventHelper {
316314
public:
317315
static v8::TracingController* GetTracingController();
318316
static void SetTracingController(v8::TracingController* controller);

test/cctest/test_platform.cc

+19
Original file line numberDiff line numberDiff line change
@@ -104,3 +104,22 @@ TEST_F(NodeZeroIsolateTestFixture, IsolatePlatformDelegateTest) {
104104
platform->UnregisterIsolate(isolate);
105105
isolate->Dispose();
106106
}
107+
108+
TEST_F(PlatformTest, TracingControllerNullptr) {
109+
v8::TracingController* orig_controller = node::GetTracingController();
110+
node::SetTracingController(nullptr);
111+
EXPECT_EQ(node::GetTracingController(), nullptr);
112+
113+
v8::Isolate::Scope isolate_scope(isolate_);
114+
const v8::HandleScope handle_scope(isolate_);
115+
const Argv argv;
116+
Env env {handle_scope, argv};
117+
118+
node::LoadEnvironment(*env, [&](const node::StartExecutionCallbackInfo& info)
119+
-> v8::MaybeLocal<v8::Value> {
120+
return v8::Null(isolate_);
121+
});
122+
123+
node::SetTracingController(orig_controller);
124+
EXPECT_EQ(node::GetTracingController(), orig_controller);
125+
}

0 commit comments

Comments
 (0)