From 14476ff3eb6828af70ff87ac7767b95cbec52f36 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Tue, 6 Nov 2018 10:21:43 +0000 Subject: [PATCH] tracing: fix static destruction order issue Sometimes, the `parallel/test-tracing-no-crash` would not work as expected, at least on Windows, because there is a static destruction race between tearing down the `NodeTraceWriter` instance and the per-process options struct. If the per-process options were destroyed before the `NodeTraceWriter`, the reference to the tracing filename would be gone before opening the file was attempted. This can be solved by creating a copy of the string when creating the `NodeTraceWriter` instance rather than taking a reference. Fixes: https://github.com/nodejs/node/issues/22523 --- src/tracing/node_trace_writer.h | 2 +- test/parallel/test-tracing-no-crash.js | 14 +++++++++++--- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/src/tracing/node_trace_writer.h b/src/tracing/node_trace_writer.h index 5e5781479c689f..a91176ad4905a9 100644 --- a/src/tracing/node_trace_writer.h +++ b/src/tracing/node_trace_writer.h @@ -61,7 +61,7 @@ class NodeTraceWriter : public AsyncTraceWriter { int highest_request_id_completed_ = 0; int total_traces_ = 0; int file_num_ = 0; - const std::string& log_file_pattern_; + std::string log_file_pattern_; std::ostringstream stream_; std::unique_ptr json_trace_writer_; bool exited_ = false; diff --git a/test/parallel/test-tracing-no-crash.js b/test/parallel/test-tracing-no-crash.js index 5b11522911e75d..0ae402f5288cca 100644 --- a/test/parallel/test-tracing-no-crash.js +++ b/test/parallel/test-tracing-no-crash.js @@ -8,7 +8,15 @@ function CheckNoSignalAndErrorCodeOne(code, signal) { assert.strictEqual(code, 1); } -const child = spawn(process.execPath, - ['--trace-event-categories', 'madeup', '-e', - 'throw new Error()'], { stdio: 'inherit' }); +const child = spawn(process.execPath, [ + '--trace-event-categories', 'madeup', '-e', 'throw new Error()' +], { stdio: [ 'inherit', 'inherit', 'pipe' ] }); child.on('exit', common.mustCall(CheckNoSignalAndErrorCodeOne)); + +let stderr; +child.stderr.setEncoding('utf8'); +child.stderr.on('data', (chunk) => stderr += chunk); +child.stderr.on('end', common.mustCall(() => { + assert(stderr.includes('throw new Error()'), stderr); + assert(!stderr.includes('Could not open trace file'), stderr); +}));