FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

cli: add `--trace-atomics-wait` flag · nodejs/node@2dc5db8 · GitHub

/ node Public

Commit 2dc5db8

Browse files
addaleax
authored and
codebytere
committed
cli: add --trace-atomics-wait flag
Adds a flag that helps with debugging deadlocks due to incorrectly implemented `Atomics.wait()` calls. PR-URL: #33292 Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: David Carlier <devnexen@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
1 parent c7c420e commit 2dc5db8

7 files changed

Lines changed: 178 additions & 0 deletions

File tree

‎benchmark/worker/atomics-wait.js‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
'use strict';
2+
/* global SharedArrayBuffer */
3+
4+
const common = require('../common.js');
5+
const bench = common.createBenchmark(main, {
6+
n: [1e7]
7+
});
8+
9+
function main({ n }) {
10+
const i32arr = new Int32Array(new SharedArrayBuffer(4));
11+
bench.start();
12+
for (let i = 0; i < n; i++)
13+
Atomics.wait(i32arr, 0, 1); // Will return immediately.
14+
bench.end(n);
15+
}

‎doc/api/cli.md‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -814,6 +814,33 @@ added: v12.0.0
814814
Set default [`tls.DEFAULT_MIN_VERSION`][] to 'TLSv1.3'. Use to disable support
815815
for TLSv1.2, which is not as secure as TLSv1.3.
816816

817+
### `--trace-atomics-wait`
818+
<!-- YAML
819+
added: REPLACEME
820+
-->
821+
822+
Print short summaries of calls to [`Atomics.wait()`][] to stderr.
823+
The output could look like this:
824+
825+
```text
826+
(node:15701) [Thread 0] Atomics.wait(<address> + 0, 1, inf) started
827+
(node:15701) [Thread 0] Atomics.wait(<address> + 0, 1, inf) did not wait because the values mismatched
828+
(node:15701) [Thread 0] Atomics.wait(<address> + 0, 0, 10) started
829+
(node:15701) [Thread 0] Atomics.wait(<address> + 0, 0, 10) timed out
830+
(node:15701) [Thread 0] Atomics.wait(<address> + 4, 0, inf) started
831+
(node:15701) [Thread 1] Atomics.wait(<address> + 4, -1, inf) started
832+
(node:15701) [Thread 0] Atomics.wait(<address> + 4, 0, inf) was woken up by another thread
833+
(node:15701) [Thread 1] Atomics.wait(<address> + 4, -1, inf) was woken up by another thread
834+
```
835+
836+
The fields here correspond to:
837+
838+
* The thread id as given by [`worker_threads.threadId`][]
839+
* The base address of the `SharedArrayBuffer` in question, as well as the
840+
byte offset corresponding to the index passed to `Atomics.wait()`
841+
* The expected value that was passed to `Atomics.wait()`
842+
* The timeout passed to `Atomics.wait`
843+
817844
### `--trace-deprecation`
818845
<!-- YAML
819846
added: v0.8.0
@@ -1203,6 +1230,7 @@ Node.js options that are allowed are:
12031230
* `--tls-min-v1.1`
12041231
* `--tls-min-v1.2`
12051232
* `--tls-min-v1.3`
1233+
* `--trace-atomics-wait`
12061234
* `--trace-deprecation`
12071235
* `--trace-event-categories`
12081236
* `--trace-event-file-pattern`
@@ -1472,12 +1500,14 @@ $ node --max-old-space-size=1536 index.js
14721500
```
14731501

14741502
[`--openssl-config`]: #cli_openssl_config_file
1503+
[`Atomics.wait()`]: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Atomics/wait
14751504
[`Buffer`]: buffer.html#buffer_class_buffer
14761505
[`SlowBuffer`]: buffer.html#buffer_class_slowbuffer
14771506
[`process.setUncaughtExceptionCaptureCallback()`]: process.html#process_process_setuncaughtexceptioncapturecallback_fn
14781507
[`tls.DEFAULT_MAX_VERSION`]: tls.html#tls_tls_default_max_version
14791508
[`tls.DEFAULT_MIN_VERSION`]: tls.html#tls_tls_default_min_version
14801509
[`unhandledRejection`]: process.html#process_event_unhandledrejection
1510+
[`worker_threads.threadId`]: worker_threads.html##worker_threads_worker_threadid
14811511
[Chrome DevTools Protocol]: https://chromedevtools.github.io/devtools-protocol/
14821512
[REPL]: repl.html
14831513
[ScriptCoverage]: https://chromedevtools.github.io/devtools-protocol/tot/Profiler#type-ScriptCoverage

‎doc/node.1‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,10 @@ but the option is supported for compatibility with older Node.js versions.
363363
Set default minVersion to 'TLSv1.3'. Use to disable support for TLSv1.2 in
364364
favour of TLSv1.3, which is more secure.
365365
.
366+
.It Fl -trace-atomics-wait
367+
Print short summaries of calls to
368+
.Sy Atomics.wait() .
369+
.
366370
.It Fl -trace-deprecation
367371
Print stack traces for deprecations.
368372
.

‎src/node.cc‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,11 +227,56 @@ int Environment::InitializeInspector(
227227
}
228228
#endif // HAVE_INSPECTOR && NODE_USE_V8_PLATFORM
229229

230+
#define ATOMIC_WAIT_EVENTS(V) \
231+
V(kStartWait, "started") \
232+
V(kWokenUp, "was woken up by another thread") \
233+
V(kTimedOut, "timed out") \
234+
V(kTerminatedExecution, "was stopped by terminated execution") \
235+
V(kAPIStopped, "was stopped through the embedder API") \
236+
V(kNotEqual, "did not wait because the values mismatched") \
237+
238+
static void AtomicsWaitCallback(Isolate::AtomicsWaitEvent event,
239+
Local<v8::SharedArrayBuffer> array_buffer,
240+
size_t offset_in_bytes, int64_t value,
241+
double timeout_in_ms,
242+
Isolate::AtomicsWaitWakeHandle* stop_handle,
243+
void* data) {
244+
Environment* env = static_cast<Environment*>(data);
245+
246+
const char* message;
247+
switch (event) {
248+
#define V(key, msg) \
249+
case Isolate::AtomicsWaitEvent::key: \
250+
message = msg; \
251+
break;
252+
ATOMIC_WAIT_EVENTS(V)
253+
#undef V
254+
}
255+
256+
fprintf(stderr,
257+
"(node:%d) [Thread %" PRIu64 "] Atomics.wait(%p + %zx, %" PRId64
258+
", %.f) %s\n",
259+
static_cast<int>(uv_os_getpid()),
260+
env->thread_id(),
261+
array_buffer->GetBackingStore()->Data(),
262+
offset_in_bytes,
263+
value,
264+
timeout_in_ms,
265+
message);
266+
}
267+
230268
void Environment::InitializeDiagnostics() {
231269
isolate_->GetHeapProfiler()->AddBuildEmbedderGraphCallback(
232270
Environment::BuildEmbedderGraph, this);
233271
if (options_->trace_uncaught)
234272
isolate_->SetCaptureStackTraceForUncaughtExceptions(true);
273+
if (options_->trace_atomics_wait) {
274+
isolate_->SetAtomicsWaitCallback(AtomicsWaitCallback, this);
275+
AddCleanupHook([](void* data) {
276+
Environment* env = static_cast<Environment*>(data);
277+
env->isolate()->SetAtomicsWaitCallback(nullptr, nullptr);
278+
}, this);
279+
}
235280

236281
#if defined HAVE_DTRACE || defined HAVE_ETW
237282
InitDTrace(this);

‎src/node_options.cc‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,10 @@ EnvironmentOptionsParser::EnvironmentOptionsParser() {
435435
"throw an exception on deprecations",
436436
&EnvironmentOptions::throw_deprecation,
437437
kAllowedInEnvironment);
438+
AddOption("--trace-atomics-wait",
439+
"trace Atomics.wait() operations",
440+
&EnvironmentOptions::trace_atomics_wait,
441+
kAllowedInEnvironment);
438442
AddOption("--trace-deprecation",
439443
"show stack traces on deprecations",
440444
&EnvironmentOptions::trace_deprecation,

‎src/node_options.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,7 @@ class EnvironmentOptions : public Options {
140140
std::string redirect_warnings;
141141
bool test_udp_no_try_send = false;
142142
bool throw_deprecation = false;
143+
bool trace_atomics_wait = false;
143144
bool trace_deprecation = false;
144145
bool trace_exit = false;
145146
bool trace_sync_io = false;

‎test/parallel/test-trace-atomics-wait.js‎

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,79 @@
1+
'use strict';
2+
require('../common');
3+
const assert = require('assert');
4+
const child_process = require('child_process');
5+
const { Worker } = require('worker_threads');
6+
7+
if (process.argv[2] === 'child') {
8+
const i32arr = new Int32Array(new SharedArrayBuffer(8));
9+
assert.strictEqual(Atomics.wait(i32arr, 0, 1), 'not-equal');
10+
assert.strictEqual(Atomics.wait(i32arr, 0, 0, 10), 'timed-out');
11+
12+
new Worker(`
13+
const i32arr = require('worker_threads').workerData;
14+
Atomics.store(i32arr, 1, -1);
15+
Atomics.notify(i32arr, 1);
16+
Atomics.wait(i32arr, 1, -1);
17+
`, { eval: true, workerData: i32arr });
18+
19+
Atomics.wait(i32arr, 1, 0);
20+
assert.strictEqual(Atomics.load(i32arr, 1), -1);
21+
Atomics.store(i32arr, 1, 0);
22+
Atomics.notify(i32arr, 1);
23+
return;
24+
}
25+
26+
const proc = child_process.spawnSync(
27+
process.execPath,
28+
[ '--trace-atomics-wait', __filename, 'child' ],
29+
{ encoding: 'utf8', stdio: [ 'inherit', 'inherit', 'pipe' ] });
30+
31+
if (proc.status !== 0) console.log(proc);
32+
assert.strictEqual(proc.status, 0);
33+
34+
const SABAddress = proc.stderr.match(/Atomics\.wait\((?<SAB>.+) \+/).groups.SAB;
35+
const actualTimeline = proc.stderr
36+
.replace(new RegExp(SABAddress, 'g'), '<address>')
37+
.replace(new RegExp(`\\(node:${proc.pid}\\) `, 'g'), '')
38+
.replace(/\binf(inity)?\b/gi, 'inf')
39+
.replace(/\r/g, '')
40+
.trim();
41+
console.log('+++ normalized stdout +++');
42+
console.log(actualTimeline);
43+
console.log('--- normalized stdout ---');
44+
45+
const begin =
46+
`[Thread 0] Atomics.wait(<address> + 0, 1, inf) started
47+
[Thread 0] Atomics.wait(<address> + 0, 1, inf) did not wait because the \
48+
values mismatched
49+
[Thread 0] Atomics.wait(<address> + 0, 0, 10) started
50+
[Thread 0] Atomics.wait(<address> + 0, 0, 10) timed out`;
51+
52+
const expectedTimelines = [
53+
`${begin}
54+
[Thread 0] Atomics.wait(<address> + 4, 0, inf) started
55+
[Thread 1] Atomics.wait(<address> + 4, -1, inf) started
56+
[Thread 0] Atomics.wait(<address> + 4, 0, inf) was woken up by another thread
57+
[Thread 1] Atomics.wait(<address> + 4, -1, inf) was woken up by another thread`,
58+
`${begin}
59+
[Thread 0] Atomics.wait(<address> + 4, 0, inf) started
60+
[Thread 0] Atomics.wait(<address> + 4, 0, inf) was woken up by another thread
61+
[Thread 1] Atomics.wait(<address> + 4, -1, inf) started
62+
[Thread 1] Atomics.wait(<address> + 4, -1, inf) did not wait because the \
63+
values mismatched`,
64+
`${begin}
65+
[Thread 0] Atomics.wait(<address> + 4, 0, inf) started
66+
[Thread 1] Atomics.wait(<address> + 4, -1, inf) started
67+
[Thread 0] Atomics.wait(<address> + 4, 0, inf) was woken up by another thread
68+
[Thread 1] Atomics.wait(<address> + 4, -1, inf) did not wait because the \
69+
values mismatched`,
70+
`${begin}
71+
[Thread 0] Atomics.wait(<address> + 4, 0, inf) started
72+
[Thread 0] Atomics.wait(<address> + 4, 0, inf) did not wait because the \
73+
values mismatched
74+
[Thread 1] Atomics.wait(<address> + 4, -1, inf) started
75+
[Thread 1] Atomics.wait(<address> + 4, -1, inf) did not wait because the \
76+
values mismatched`
77+
];
78+
79+
assert(expectedTimelines.includes(actualTimeline));

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL