Skip to content

Commit 66094c3

Browse files
committed
squash! tracing: forbid tracing modifications from worker threads
1 parent 443bde0 commit 66094c3

File tree

4 files changed

+44
-2
lines changed

4 files changed

+44
-2
lines changed

doc/api/tracing.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,8 @@ as the one used by `process.hrtime()`
8282
however the trace-event timestamps are expressed in microseconds,
8383
unlike `process.hrtime()` which returns nanoseconds.
8484

85+
The features from this module are not available in [`Worker`][] threads.
86+
8587
## The `trace_events` module
8688
<!-- YAML
8789
added: v10.0.0
@@ -205,3 +207,4 @@ console.log(trace_events.getEnabledCategories());
205207
[Performance API]: perf_hooks.html
206208
[V8]: v8.html
207209
[`async_hooks`]: async_hooks.html
210+
[`Worker`]: worker_threads.html#worker_threads_class_worker

src/env.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -131,7 +131,7 @@ void Environment::TrackingTraceStateObserver::UpdateTraceCategoryState() {
131131
if (!env_->is_main_thread()) {
132132
// Ideally, we’d have a consistent story that treats all threads/Environment
133133
// instances equally here. However, tracing is essentially global, and this
134-
// callback is callback from whichever thread calls `StartTracing()` or
134+
// callback is called from whichever thread calls `StartTracing()` or
135135
// `StopTracing()`. The only way to do this in a threadsafe fashion
136136
// seems to be only tracking this from the main thread, and only allowing
137137
// these state modifications from the main thread.
@@ -192,7 +192,7 @@ Environment::Environment(IsolateData* isolate_data,
192192
AssignToContext(context, ContextInfo(""));
193193

194194
if (tracing::AgentWriterHandle* writer = GetTracingAgentWriter()) {
195-
trace_state_observer_.reset(new TrackingTraceStateObserver(this));
195+
trace_state_observer_ = std::make_unique<TrackingTraceStateObserver>(this);
196196
v8::TracingController* tracing_controller = writer->GetTracingController();
197197
if (tracing_controller != nullptr)
198198
tracing_controller->AddTraceStateObserver(trace_state_observer_.get());
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// Flags: --experimental-worker
2+
'use strict';
3+
4+
const common = require('../common');
5+
const { Worker } = require('worker_threads');
6+
7+
new Worker("require('trace_events')", { eval: true })
8+
.on('error', common.expectsError({
9+
code: 'ERR_TRACE_EVENTS_UNAVAILABLE',
10+
type: Error
11+
}));
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
// Flags: --experimental-worker
2+
'use strict';
3+
4+
const common = require('../common');
5+
const { Worker } = require('worker_threads');
6+
7+
common.skipIfInspectorDisabled();
8+
9+
if (!process.env.HAS_STARTED_WORKER) {
10+
process.env.HAS_STARTED_WORKER = 1;
11+
new Worker(__filename);
12+
return;
13+
}
14+
15+
const assert = require('assert');
16+
const { Session } = require('inspector');
17+
18+
const session = new Session();
19+
session.connect();
20+
session.post('NodeTracing.start', {
21+
traceConfig: { includedCategories: ['node.perf'] }
22+
}, common.mustCall((err) => {
23+
assert.deepStrictEqual(err, {
24+
code: -32000,
25+
message:
26+
'Tracing properties can only be changed through main thread sessions'
27+
});
28+
}));

0 commit comments

Comments
 (0)