Skip to content

Commit 4f4245d

Browse files
joyeecheungaduh95
joyeecheung
authored and
aduh95
committed
debugger: add --max-hit option to probe mode
This option provides a new way of terminating the probing by specifying a limit for the per-probe hit evaluation. When any probe reaches the limit, the session removes the V8 breakpoints and finishes with a `completed` terminal event rather than waiting for the target to exit or timeout. This would be useful when we provide a way to attach to and probe a running process in the future. Signed-off-by: Joyee Cheung <[email protected]> PR-URL: #63704 Refs: #63646 Reviewed-By: Jan Martin <[email protected]> Reviewed-By: Chengzhong Wu <[email protected]>
1 parent 757950a commit 4f4245d

12 files changed

Lines changed: 358 additions & 15 deletions

‎doc/api/debugger.md‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,10 @@ debug>
236236
added:
237237
- v24.16.0
238238
changes:
239+
- version: REPLACEME
240+
pr-url: https://github.com/nodejs/node/pull/63704
241+
description: Add per-probe `--max-hit <n>` option to limit evaluated hits and finish
242+
with a `completed` terminal event as soon as any probe reaches its limit.
239243
- version: REPLACEME
240244
pr-url: https://github.com/nodejs/node/pull/63437
241245
description: Add `probe_failure` terminal `error` event for inspector-side mid-session
@@ -264,8 +268,8 @@ printf-style debugging without having to modify the application code and
264268
clean up afterwards. It also supports structured JSON output for tool use.
265269

266270
```console
267-
$ node inspect --probe <file>:<line>[:<col>] --expr <expr>
268-
[--probe <file>:<line>[:<col>] --expr <expr> ...]
271+
$ node inspect --probe <file>:<line>[:<col>] --expr <expr> [--max-hit <n>]
272+
[--probe <file>:<line>[:<col>] --expr <expr> [--max-hit <n>] ...]
269273
[--json] [--preview] [--timeout=<ms>] [--port=<port>]
270274
[--] [<node-option> ...] <script> [<script-args> ...]
271275
```
@@ -278,6 +282,11 @@ $ node inspect --probe <file>:<line>[:<col>] --expr <expr>
278282
* `--expr <expr>`: JavaScript expression to evaluate whenever execution reaches
279283
the location specified by the preceding `--probe`.
280284
Must immediately follow the `--probe` it belongs to.
285+
* `--max-hit <n>`: An optional per-probe limit on the number of times the probe
286+
can be hit. When not specified, there's no hit limit. When any probe reaches
287+
its hit limit, the probing process will detach and report the results. The process
288+
being probed will continue to run. If any other probe is never reached by the time
289+
the session ends, it will be reported as a missed probe.
281290
* `--timeout=<ms>`: A global wall-clock deadline for the entire probe session.
282291
The default is `30000`. This can be used to probe a long-running application
283292
that can be terminated externally.
@@ -292,6 +301,10 @@ Additional rules about the `--probe` and `--expr` arguments:
292301

293302
* `--probe <file>:<line>[:<col>]` and `--expr <expr>` are strict pairs. Each
294303
`--probe` must be followed immediately by exactly one `--expr`.
304+
* `--max-hit <n>` is an optional per-probe option that applies to the most recent
305+
`--probe`/`--expr` pair. It may not appear before the first `--probe` or
306+
between a `--probe` and its matching `--expr`, and may be given at most once
307+
per probe.
295308
* `--timeout`, `--json`, `--preview`, and `--port` are global probe options
296309
for the whole probe session. They may appear before or between probe pairs,
297310
but not between a `--probe` and its matching `--expr`.
@@ -366,6 +379,7 @@ $ node inspect --json --probe cli.js:5 --expr 'rss' cli.js
366379
"suffix": "cli.js",
367380
"line": 5
368381
}
382+
// `maxHit` is present only when the probe was given a --max-hit limit.
369383
}
370384
],
371385
"results": [

‎lib/internal/debugger/inspect.js‎

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -267,16 +267,17 @@ function parseInteractiveArgs(args) {
267267
}
268268

269269
const kInspectArgOptions = {
270-
__proto__: null,
271-
expr: { type: 'string' },
272-
help: { type: 'boolean', short: 'h' },
273-
json: { type: 'boolean' },
274-
// Port and timeout use type 'string' because parseArgs has no
270+
'__proto__': null,
271+
'expr': { type: 'string' },
272+
'help': { type: 'boolean', short: 'h' },
273+
'json': { type: 'boolean' },
274+
// Port, timeout, and max-hit use type 'string' because parseArgs has no
275275
// numeric type; the values are parsed to integers by parseProbeTokens().
276-
port: { type: 'string' },
277-
preview: { type: 'boolean' },
278-
probe: { type: 'string' },
279-
timeout: { type: 'string' },
276+
'max-hit': { type: 'string' },
277+
'port': { type: 'string' },
278+
'preview': { type: 'boolean' },
279+
'probe': { type: 'string' },
280+
'timeout': { type: 'string' },
280281
};
281282

282283
// Parses args once and decides whether the user wants the inspect help, probe

‎lib/internal/debugger/inspect_helpers.js‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,8 +69,8 @@ function writeInspectUsageAndExit(invokedAs, message, exitCode) {
6969
}
7070
out.write(`Usage: ${invokedAs} [--port=<port>] [<node-option> ...]
7171
[<script> [<script-args>] | <host>:<port> | -p <pid>]
72-
${invokedAs} --probe <file>:<line>[:<col>] --expr <expr>
73-
[--probe <file>:<line>[:<col>] --expr <expr> ...]
72+
${invokedAs} --probe <file>:<line>[:<col>] --expr <expr> [--max-hit <n>]
73+
[--probe <file>:<line>[:<col>] --expr <expr> [--max-hit <n>] ...]
7474
[--json] [--preview] [--timeout=<ms>] [--port=<port>]
7575
[--] [<node-option> ...] <script> [<script-args> ...]
7676
@@ -109,6 +109,10 @@ Options:
109109
preceding --probe each time execution reaches it.
110110
Avoid probing let/const-bound variables at their
111111
declaration site or a ReferenceError may be thrown.
112+
--max-hit <n> Per-probe limit on evaluated hits. When not specified,
113+
there's no hit limit. When any probe reaches its hit LIMIT,
114+
the probing process will detach and report the results.
115+
The probed process will continue to run.
112116
--json Output JSON if specified, otherwise human-readable text.
113117
--preview Include V8 object previews in JSON output.
114118
--timeout <ms> Global session timeout (default: 30000).

‎lib/internal/debugger/inspect_probe.js‎

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ const {
77
ArrayPrototypeMap,
88
ArrayPrototypePush,
99
ArrayPrototypeSlice,
10+
ArrayPrototypeSome,
1011
FunctionPrototypeBind,
1112
JSONStringify,
1213
NumberIsNaN,
@@ -86,9 +87,14 @@ const kInspectPortRegex = /^--inspect-port=(\d+)$/;
8687
* @typedef {object} Probe
8788
* @property {string} expr Expression to evaluate on hit.
8889
* @property {ProbeTarget} target User's original --probe request shape.
90+
* @property {number} maxHit Per-probe hit limit from --max-hit. Infinity when unlimited.
8991
* @property {number} hits Count of hits observed.
9092
*/
9193

94+
function probeReachedLimit(probe) {
95+
return probe.hits >= probe.maxHit;
96+
}
97+
9298
function parseUnsignedInteger(value, name, allowZero = false) {
9399
if (typeof value !== 'string' || RegExpPrototypeExec(kDigitsRegex, value) === null) {
94100
throw new ERR_DEBUGGER_STARTUP_ERROR(`Invalid ${name}: ${value}`);
@@ -371,6 +377,20 @@ function parseProbeTokens(tokens, args) {
371377
break;
372378
case 'expr':
373379
throw new ERR_DEBUGGER_STARTUP_ERROR('Unexpected --expr before --probe');
380+
case 'max-hit': {
381+
if (probes.length === 0) {
382+
throw new ERR_DEBUGGER_STARTUP_ERROR('Unexpected --max-hit before --probe');
383+
}
384+
if (token.value === undefined) {
385+
throw new ERR_DEBUGGER_STARTUP_ERROR(`Missing value for ${token.rawName}`);
386+
}
387+
const probe = probes[probes.length - 1];
388+
if (probe.maxHit !== undefined) {
389+
throw new ERR_DEBUGGER_STARTUP_ERROR('Duplicate --max-hit for a single --probe');
390+
}
391+
probe.maxHit = parseUnsignedInteger(token.value, 'max-hit');
392+
break;
393+
}
374394
default:
375395
if (probes.length > 0) {
376396
throw new ERR_DEBUGGER_STARTUP_ERROR(
@@ -458,7 +478,9 @@ class ProbeInspectorSession {
458478
this.completionPromise = promise;
459479
this.resolveCompletion = resolve;
460480
/** @type {Probe[]} */
461-
this.probes = ArrayPrototypeMap(options.probes, ({ expr, target }) => ({ expr, target, hits: 0 }));
481+
this.probes = ArrayPrototypeMap(options.probes,
482+
({ expr, target, maxHit }) =>
483+
({ expr, target, maxHit: maxHit ?? Infinity, hits: 0 }));
462484
this.onChildOutput = FunctionPrototypeBind(this.onChildOutput, this);
463485
this.onChildExit = FunctionPrototypeBind(this.onChildExit, this);
464486
this.onClientClose = FunctionPrototypeBind(this.onClientClose, this);
@@ -642,6 +664,15 @@ class ProbeInspectorSession {
642664
}
643665
}
644666

667+
// Finish proactively as soon as any probe reaches its hit limit. All probes
668+
// hit in this pause are recorded first, then the session ends.
669+
// TODO(joyeecheung): When we implement attach mode, this teardown must
670+
// resume-and-detach rather than kill, since the target is not ours.
671+
if (!this.finished && ArrayPrototypeSome(this.probes, probeReachedLimit)) {
672+
this.finishWithTrustedResult({ event: 'completed' });
673+
return;
674+
}
675+
645676
await this.resume();
646677
}
647678

@@ -912,7 +943,12 @@ class ProbeInspectorSession {
912943
code: exitCode,
913944
report: {
914945
v: kProbeVersion,
915-
probes: ArrayPrototypeMap(this.probes, ({ expr, target }) => ({ expr, target })),
946+
probes: ArrayPrototypeMap(this.probes, ({ expr, target, maxHit }) => {
947+
// Omit an unlimited maxHit, as Infinity would serialize to null in JSON.
948+
const probe = { expr, target };
949+
if (maxHit !== Infinity) { probe.maxHit = maxHit; }
950+
return probe;
951+
}),
916952
results,
917953
},
918954
};
@@ -931,6 +967,8 @@ class ProbeInspectorSession {
931967

932968
if (this.child === null) { return; }
933969

970+
// TODO(joyeecheung): When we implement attach mode, this teardown must
971+
// resume-and-detach rather than kill, since the target is not ours.
934972
if (this.child.exitCode === null && this.child.signalCode === null) {
935973
this.child.kill();
936974
}

‎test/common/debugger-probe.js‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
'use strict';
22

33
const assert = require('assert');
4+
const { spawnSyncAndExit } = require('./child_process');
45

56
// Work around a pre-existing inspector issue: if the debuggee exits too quickly
67
// the inspector can segfault while tearing down. For now normalize the segfault
@@ -79,7 +80,17 @@ function assertProbeText(output, expected) {
7980
assert.strictEqual(normalized, expected);
8081
}
8182

83+
function assertProbeCliError(inspectArgs, expectedStderr, { cwd } = {}) {
84+
spawnSyncAndExit(process.execPath, ['inspect', ...inspectArgs], { cwd }, {
85+
signal: null,
86+
status: 9,
87+
stderr: expectedStderr,
88+
trim: true,
89+
});
90+
}
91+
8292
module.exports = {
8393
assertProbeJson,
94+
assertProbeCliError,
8495
assertProbeText,
8596
};

‎test/fixtures/debugger/probe-max-hit.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
'use strict';
2+
3+
let total = 0;
4+
for (let index = 0; index < 3; index++) {
5+
total += index + 1;
6+
}
7+
console.log(total);

‎test/parallel/test-debugger-probe-max-hit-invalid.js‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
// This tests that probe mode rejects malformed --max-hit usage.
2+
'use strict';
3+
4+
const common = require('../common');
5+
common.skipIfInspectorDisabled();
6+
7+
const fixtures = require('../common/fixtures');
8+
const { assertProbeCliError } = require('../common/debugger-probe');
9+
10+
const cwd = fixtures.path('debugger');
11+
12+
// --max-hit before any --probe.
13+
assertProbeCliError(
14+
['--max-hit', '1', '--probe', 'probe.js:12', '--expr', 'finalValue', 'probe.js'],
15+
/Unexpected --max-hit before --probe/, { cwd });
16+
17+
// --max-hit between a --probe and its --expr.
18+
assertProbeCliError(
19+
['--probe', 'probe.js:12', '--max-hit', '1', '--expr', 'finalValue', 'probe.js'],
20+
/Each --probe must be followed immediately by --expr/, { cwd });
21+
22+
// Duplicate --max-hit for a single probe.
23+
assertProbeCliError(
24+
['--probe', 'probe.js:12', '--expr', 'finalValue', '--max-hit', '1', '--max-hit', '2', 'probe.js'],
25+
/Duplicate --max-hit for a single --probe/, { cwd });
26+
27+
// Non-numeric value.
28+
assertProbeCliError(
29+
['--probe', 'probe.js:12', '--expr', 'finalValue', '--max-hit', 'abc', 'probe.js'],
30+
/Invalid max-hit: abc/, { cwd });
31+
32+
// Zero is not allowed (limit must be at least 1).
33+
assertProbeCliError(
34+
['--probe', 'probe.js:12', '--expr', 'finalValue', '--max-hit', '0', 'probe.js'],
35+
/Invalid max-hit: 0/, { cwd });
36+
37+
// Missing value: --max-hit as the final token has nothing to consume.
38+
assertProbeCliError(
39+
['--probe', 'probe.js:12', '--expr', 'finalValue', '--max-hit'],
40+
/Missing value for --max-hit/, { cwd });

‎test/parallel/test-debugger-probe-max-hit-miss.js‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
// This tests that a limited probe that is never reached is still reported as a missed probe.
2+
'use strict';
3+
4+
const common = require('../common');
5+
common.skipIfInspectorDisabled();
6+
7+
const fixtures = require('../common/fixtures');
8+
const { spawnSyncAndAssert } = require('../common/child_process');
9+
const { assertProbeJson } = require('../common/debugger-probe');
10+
11+
const cwd = fixtures.path('debugger');
12+
13+
spawnSyncAndAssert(process.execPath, [
14+
'inspect',
15+
'--json',
16+
'--probe', 'probe-miss.js:99',
17+
'--expr', '42',
18+
'--max-hit', '3',
19+
'probe-miss.js',
20+
], { cwd }, {
21+
stdout(output) {
22+
assertProbeJson(output, {
23+
v: 2,
24+
probes: [{
25+
expr: '42',
26+
target: { suffix: 'probe-miss.js', line: 99 },
27+
maxHit: 3,
28+
}],
29+
results: [{
30+
event: 'miss',
31+
pending: [0],
32+
}],
33+
});
34+
},
35+
trim: true,
36+
});

‎test/parallel/test-debugger-probe-max-hit-partial.js‎

Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
// This tests that when the target exits before a probe reaches its limit,
2+
// the session still ends with `completed`.
3+
'use strict';
4+
5+
const common = require('../common');
6+
common.skipIfInspectorDisabled();
7+
8+
const fixtures = require('../common/fixtures');
9+
const { spawnSyncAndAssert } = require('../common/child_process');
10+
const { assertProbeJson } = require('../common/debugger-probe');
11+
12+
const cwd = fixtures.path('debugger');
13+
const probeUrl = fixtures.fileURL('debugger', 'probe-max-hit.js').href;
14+
15+
spawnSyncAndAssert(process.execPath, [
16+
'inspect',
17+
'--json',
18+
'--probe', 'probe-max-hit.js:5',
19+
'--expr', 'index',
20+
'--max-hit', '10',
21+
'probe-max-hit.js',
22+
], { cwd }, {
23+
stdout(output) {
24+
assertProbeJson(output, {
25+
v: 2,
26+
probes: [{
27+
expr: 'index',
28+
target: { suffix: 'probe-max-hit.js', line: 5 },
29+
maxHit: 10,
30+
}],
31+
results: [
32+
{
33+
probe: 0,
34+
event: 'hit',
35+
hit: 1,
36+
location: { url: probeUrl, line: 5, column: 3 },
37+
result: { type: 'number', value: 0, description: '0' },
38+
},
39+
{
40+
probe: 0,
41+
event: 'hit',
42+
hit: 2,
43+
location: { url: probeUrl, line: 5, column: 3 },
44+
result: { type: 'number', value: 1, description: '1' },
45+
},
46+
{
47+
probe: 0,
48+
event: 'hit',
49+
hit: 3,
50+
location: { url: probeUrl, line: 5, column: 3 },
51+
result: { type: 'number', value: 2, description: '2' },
52+
},
53+
{ event: 'completed' },
54+
],
55+
});
56+
},
57+
trim: true,
58+
});

0 commit comments

Comments
 (0)