From 66094c3c716e194ddb413ef72007d2b7b44ee721 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
Date: Sun, 21 Oct 2018 03:43:22 +0200
Subject: [PATCH] squash! tracing: forbid tracing modifications from worker
 threads

---
 doc/api/tracing.md                            |  3 ++
 src/env.cc                                    |  4 +--
 .../test-trace-events-api-worker-disabled.js  | 11 ++++++++
 ...-events-dynamic-enable-workers-disabled.js | 28 +++++++++++++++++++
 4 files changed, 44 insertions(+), 2 deletions(-)
 create mode 100644 test/parallel/test-trace-events-api-worker-disabled.js
 create mode 100644 test/parallel/test-trace-events-dynamic-enable-workers-disabled.js

diff --git a/doc/api/tracing.md b/doc/api/tracing.md
index 04db3f12f42e2e..10b2f61ea5a5b9 100644
--- a/doc/api/tracing.md
+++ b/doc/api/tracing.md
@@ -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
@@ -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
diff --git a/src/env.cc b/src/env.cc
index 527058766ac86a..51e79ec3601931 100644
--- a/src/env.cc
+++ b/src/env.cc
@@ -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.
@@ -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());
diff --git a/test/parallel/test-trace-events-api-worker-disabled.js b/test/parallel/test-trace-events-api-worker-disabled.js
new file mode 100644
index 00000000000000..6140d3a83c5339
--- /dev/null
+++ b/test/parallel/test-trace-events-api-worker-disabled.js
@@ -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
+  }));
diff --git a/test/parallel/test-trace-events-dynamic-enable-workers-disabled.js b/test/parallel/test-trace-events-dynamic-enable-workers-disabled.js
new file mode 100644
index 00000000000000..adae50057e8a4e
--- /dev/null
+++ b/test/parallel/test-trace-events-dynamic-enable-workers-disabled.js
@@ -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'
+  });
+}));