Skip to content

Commit

Permalink
squash! tracing: forbid tracing modifications from worker threads
Browse files Browse the repository at this point in the history
  • Loading branch information
addaleax committed Oct 21, 2018
1 parent 443bde0 commit 66094c3
Show file tree
Hide file tree
Showing 4 changed files with 44 additions and 2 deletions.
3 changes: 3 additions & 0 deletions doc/api/tracing.md
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,8 @@ as the one used by `process.hrtime()`
however the trace-event timestamps are expressed in microseconds,
unlike `process.hrtime()` which returns nanoseconds.

The features from this module are not available in [`Worker`][] threads.

## The `trace_events` module
<!-- YAML
added: v10.0.0
Expand Down Expand Up @@ -205,3 +207,4 @@ console.log(trace_events.getEnabledCategories());
[Performance API]: perf_hooks.html
[V8]: v8.html
[`async_hooks`]: async_hooks.html
[`Worker`]: worker_threads.html#worker_threads_class_worker
4 changes: 2 additions & 2 deletions src/env.cc
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ void Environment::TrackingTraceStateObserver::UpdateTraceCategoryState() {
if (!env_->is_main_thread()) {
// Ideally, we’d have a consistent story that treats all threads/Environment
// instances equally here. However, tracing is essentially global, and this
// callback is callback from whichever thread calls `StartTracing()` or
// callback is called from whichever thread calls `StartTracing()` or
// `StopTracing()`. The only way to do this in a threadsafe fashion
// seems to be only tracking this from the main thread, and only allowing
// these state modifications from the main thread.
Expand Down Expand Up @@ -192,7 +192,7 @@ Environment::Environment(IsolateData* isolate_data,
AssignToContext(context, ContextInfo(""));

if (tracing::AgentWriterHandle* writer = GetTracingAgentWriter()) {
trace_state_observer_.reset(new TrackingTraceStateObserver(this));
trace_state_observer_ = std::make_unique<TrackingTraceStateObserver>(this);
v8::TracingController* tracing_controller = writer->GetTracingController();
if (tracing_controller != nullptr)
tracing_controller->AddTraceStateObserver(trace_state_observer_.get());
Expand Down
11 changes: 11 additions & 0 deletions test/parallel/test-trace-events-api-worker-disabled.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
// Flags: --experimental-worker
'use strict';

const common = require('../common');
const { Worker } = require('worker_threads');

new Worker("require('trace_events')", { eval: true })
.on('error', common.expectsError({
code: 'ERR_TRACE_EVENTS_UNAVAILABLE',
type: Error
}));
28 changes: 28 additions & 0 deletions test/parallel/test-trace-events-dynamic-enable-workers-disabled.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Flags: --experimental-worker
'use strict';

const common = require('../common');
const { Worker } = require('worker_threads');

common.skipIfInspectorDisabled();

if (!process.env.HAS_STARTED_WORKER) {
process.env.HAS_STARTED_WORKER = 1;
new Worker(__filename);
return;
}

const assert = require('assert');
const { Session } = require('inspector');

const session = new Session();
session.connect();
session.post('NodeTracing.start', {
traceConfig: { includedCategories: ['node.perf'] }
}, common.mustCall((err) => {
assert.deepStrictEqual(err, {
code: -32000,
message:
'Tracing properties can only be changed through main thread sessions'
});
}));

0 comments on commit 66094c3

Please sign in to comment.