debugger: add --max-hit option to probe mode · nodejs/node@4438cb5 · GitHub
Skip to content

Commit 4438cb5

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 <joyeec9h3@gmail.com> PR-URL: #63704 Refs: #63646 Reviewed-By: Jan Martin <jan.krems@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
1 parent 3ed287a commit 4438cb5

12 files changed

Lines changed: 358 additions & 15 deletions

‎doc/api/debugger.md‎

Lines changed: 16 additions & 2 deletions

‎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);
@@ -638,6 +660,15 @@ class ProbeInspectorSession {
638660
}
639661
}
640662

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

@@ -908,7 +939,12 @@ class ProbeInspectorSession {
908939
code: exitCode,
909940
report: {
910941
v: kProbeVersion,
911-
probes: ArrayPrototypeMap(this.probes, ({ expr, target }) => ({ expr, target })),
942+
probes: ArrayPrototypeMap(this.probes, ({ expr, target, maxHit }) => {
943+
// Omit an unlimited maxHit, as Infinity would serialize to null in JSON.
944+
const probe = { expr, target };
945+
if (maxHit !== Infinity) { probe.maxHit = maxHit; }
946+
return probe;
947+
}),
912948
results,
913949
},
914950
};
@@ -927,6 +963,8 @@ class ProbeInspectorSession {
927963

928964
if (this.child === null) { return; }
929965

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

‎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

0 commit comments

Comments
 (0)