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

PR-URL: https://github.com/nodejs/node/pull/24123
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
This commit is contained in:
Anna Henningsen 2018-11-06 10:21:43 +00:00
parent f8c3e258fd
commit 71a4cb7d46
No known key found for this signature in database
GPG Key ID: 9C63F3A6CD2AD8F9
2 changed files with 12 additions and 4 deletions

View File

@ -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<TraceWriter> json_trace_writer_;
bool exited_ = false;

View File

@ -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);
}));