Skip to content

Commit 4438cb5

Browse files
joyeecheungaduh95
authored andcommitted
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
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,10 @@ debug>
236236
added:
237237
- v26.1.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: v26.3.0
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
constkInspectArgOptions={
270-
__proto__: null,
271-
expr: {type: 'string'},
272-
help: {type: 'boolean',short: 'h'},
273-
json: {type: 'boolean'},
274-
// Portand 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+
functionprobeReachedLimit(probe){
95+
returnprobe.hits>=probe.maxHit;
96+
}
97+
9298
functionparseUnsignedInteger(value,name,allowZero=false){
9399
if(typeofvalue!=='string'||RegExpPrototypeExec(kDigitsRegex,value)===null){
94100
thrownewERR_DEBUGGER_STARTUP_ERROR(`Invalid ${name}: ${value}`);
@@ -371,6 +377,20 @@ function parseProbeTokens(tokens, args) {
371377
break;
372378
case'expr':
373379
thrownewERR_DEBUGGER_STARTUP_ERROR('Unexpected --expr before --probe');
380+
case'max-hit': {
381+
if(probes.length===0){
382+
thrownewERR_DEBUGGER_STARTUP_ERROR('Unexpected --max-hit before --probe');
383+
}
384+
if(token.value===undefined){
385+
thrownewERR_DEBUGGER_STARTUP_ERROR(`Missing value for ${token.rawName}`);
386+
}
387+
constprobe=probes[probes.length-1];
388+
if(probe.maxHit!==undefined){
389+
thrownewERR_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
thrownewERR_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
awaitthis.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+
constprobe={ expr, target };
945+
if(maxHit!==Infinity){probe.maxHit=maxHit;}
946+
returnprobe;
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
constassert=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+
functionassertProbeCliError(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
};
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
'use strict';
2+
3+
lettotal=0;
4+
for(letindex=0;index<3;index++){
5+
total+=index+1;
6+
}
7+
console.log(total);
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+
constcommon=require('../common');
5+
common.skipIfInspectorDisabled();
6+
7+
constfixtures=require('../common/fixtures');
8+
const{ assertProbeCliError }=require('../common/debugger-probe');
9+
10+
constcwd=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-hitbefore--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--probemustbefollowedimmediatelyby--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-hitforasingle--probe/,{ cwd });
26+
27+
// Non-numeric value.
28+
assertProbeCliError(
29+
['--probe','probe.js:12','--expr','finalValue','--max-hit','abc','probe.js'],
30+
/Invalidmax-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+
/Invalidmax-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+
/Missingvaluefor--max-hit/,{ cwd });
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+
constcommon=require('../common');
5+
common.skipIfInspectorDisabled();
6+
7+
constfixtures=require('../common/fixtures');
8+
const{ spawnSyncAndAssert }=require('../common/child_process');
9+
const{ assertProbeJson }=require('../common/debugger-probe');
10+
11+
constcwd=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+
});
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+
constcommon=require('../common');
6+
common.skipIfInspectorDisabled();
7+
8+
constfixtures=require('../common/fixtures');
9+
const{ spawnSyncAndAssert }=require('../common/child_process');
10+
const{ assertProbeJson }=require('../common/debugger-probe');
11+
12+
constcwd=fixtures.path('debugger');
13+
constprobeUrl=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)