Skip to content

debugger: defer probe pause handling until startup - #63608

Merged
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
trivikr:test-debugger-probe-no-column-indent
Jun 18, 2026
Merged

debugger: defer probe pause handling until startup#63608
nodejs-github-bot merged 2 commits into
nodejs:mainfrom
trivikr:test-debugger-probe-no-column-indent

Conversation

@trivikr

@trivikrtrivikr commented May 28, 2026

Copy link
Copy Markdown
Member

Fixes a race in debugger probe mode where the startup --inspect-brk
pause could be resumed before probe breakpoints were fully installed.

The generic Debugger.paused handler resumes pauses that do not match a
probe breakpoint. During startup, that can include the initial --inspect-brk pause.
If it is resumed before bindBreakpoints() finishes, a short target script can run
past the probe location and the probe session times out.

This change ignores pause events until probe startup has completed, so the
main setup flow remains responsible for releasing the target after
breakpoints are bound.

Refs: https://github.com/nodejs/node/actions/runs/26482141780/job/77981519238

Error Log
=== release test-debugger-probe-no-column-indent ===Path: parallel/test-debugger-probe-no-column-indentError: --- stderr ---[process 40660]: --- stderr ---[process 40660]: --- stdout ---{"v":2,"probes":[{"expr":"x","target":{"suffix":"probe-indented.js","line":6}}],"results":[{"event":"timeout","pending":[0],"error":{"code":"probe_timeout","message":"Timed out after 30000ms waiting for probes: probe-indented.js:6"}}]}[process 40660]: status = 1, signal = null/Users/runner/work/node/node/dir%20with $unusual"chars?'åß∂ƒ©∆¬…`/test/common/child_process.js:112 throw error; ^Error: - process terminated with status 1, expected 0 at Object.<anonymous> (/Users/runner/work/node/node/dir%20with $unusual"chars?'åß∂ƒ©∆¬…`/test/parallel/test-debugger-probe-no-column-indent.js:14:1) at Module._compile (node:internal/modules/cjs/loader:1879:14) at Object..js (node:internal/modules/cjs/loader:2019:10) at Module.load (node:internal/modules/cjs/loader:1602:32) at Module._load (node:internal/modules/cjs/loader:1404:12) at wrapModuleLoad (node:internal/modules/cjs/loader:255:19) at Module.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:154:5) at node:internal/main/run_main_module:33:47 { options: { cwd: '/Users/runner/work/node/node/dir%20with $unusual"chars?\'åß∂ƒ©∆¬…`/test/fixtures/debugger' }, command: '/Users/runner/work/node/node/dir%20with $unusual"chars?\'åß∂ƒ©∆¬…`/out/Release/node inspect --json --probe probe-indented.js:6 --expr x probe-indented.js'}

Assisted-by: openai:gpt-5.5

@nodejs-github-botnodejs-github-bot added debugger Issues and PRs related to the debugger subsystem. needs-ci PRs that need a full CI run. labels May 28, 2026
@codecov

codecovBot commented May 28, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 90.31%. Comparing base (d8ac301) to head (6195187).
⚠️ Report is 3 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #63608 +/- ##
==========================================
- Coverage 90.31% 90.31% -0.01% 
==========================================
Files 730 730 Lines 234695 234711 +16 Branches 43966 43959 -7 ==========================================
+ Hits 211972 211986 +14 - Misses 14432 14449 +17 + Partials 8291 8276 -15 
Files with missing linesCoverage Δ
lib/internal/debugger/inspect_probe.js79.93% <100.00%> (-0.03%)⬇️

... and 43 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@trivikr
trivikrforce-pushed the test-debugger-probe-no-column-indent branch from 6195187 to 27d087cCompareJune 9, 2026 13:58
@trivikrtrivikr added the flaky-test Issues and PRs related to the tests with unstable failures on the CI. label Jun 9, 2026

@watildewatilde left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for chasing this down. All test-debugger-probe-* pass locally with no hangs.

One non-blocking note on the comment: on Linux (NODE_DEBUG=inspect_probe) I see the Break on start pause arrive after runIfWaitingForDebugger, released by Debugger.resume — so runIfWaitingForDebugger triggers that pause rather than releasing it. And since started = true is set just before that call, the guard never actually holds the pause in this ordering (confirmed with the patch applied).

Could be the timing differs on the failing platform (macOS arm64) if you happen to have a NODE_DEBUG=inspect_probe trace from a repro, it'd be good to confirm the guard is catching the startup pause there. Either way the change is safe, so feel free to land; just maybe tweak the comment wording if the release path turns out different.

@trivikrtrivikr added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Jun 15, 2026
@trivikr
trivikrforce-pushed the test-debugger-probe-no-column-indent branch from 27d087c to 0917b58CompareJune 15, 2026 14:11
@trivikr
trivikr requested a review from watildeJune 15, 2026 14:13
@trivikr

Copy link
Copy Markdown
MemberAuthor

Thank you @watilde for review and feedback.

The comment was updated in 0917b58 (this PR), and I've requested a new review.

@trivikrtrivikr added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 15, 2026
@trivikr
trivikr marked this pull request as draft June 15, 2026 20:05
@trivikr

This comment was marked as outdated.

Keep the initial --inspect-brk pause held until probe breakpoints are
bound and probe mode explicitly releases the target. This prevents the
generic pause handler from resuming user code before probes are ready.
Refs: https://github.com/nodejs/node/actions/runs/26482141780/job/77981519238
Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
@trivikr
trivikrforce-pushed the test-debugger-probe-no-column-indent branch from 0917b58 to ff60ee7CompareJune 15, 2026 23:10
@trivikr
trivikr marked this pull request as ready for review June 15, 2026 23:18
@trivikrtrivikr added review wanted PRs that need reviews. and removed review wanted PRs that need reviews. labels Jun 16, 2026
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 17, 2026
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@trivikrtrivikr added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 18, 2026
@github-actionsgithub-actionsBot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jun 18, 2026
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@trivikrtrivikr added the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Jun 18, 2026
@trivikrtrivikr added the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 18, 2026
@nodejs-github-botnodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jun 18, 2026
@nodejs-github-bot
nodejs-github-bot merged commit 1a33a8c into nodejs:mainJun 18, 2026
108 of 110 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 1a33a8c

aduh95 pushed a commit that referenced this pull request Jun 18, 2026
Keep the initial --inspect-brk pause held until probe breakpoints are
bound and probe mode explicitly releases the target. This prevents the
generic pause handler from resuming user code before probes are ready.
Refs: https://github.com/nodejs/node/actions/runs/26482141780/job/77981519238
Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
PR-URL: #63608
Refs: https://github.com/nodejs/node/actions/runs/26482141780/job/77981519238
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
@trivikr
trivikr deleted the test-debugger-probe-no-column-indent branch June 20, 2026 03:08
@joyeecheung

joyeecheung commented Jun 24, 2026

Copy link
Copy Markdown
Member

I don't think the stated scenario can actually happen, and the change can make the behavior worse:

  1. this.started is only set to trueafterawait this.bindBreakpoints(), and the target is only released by Runtime.runIfWaitingForDebugger()after that. So no pause (not even the --inspect-brk break-on-start) can realistically arrive while !this.started - as pointed out in debugger: defer probe pause handling until startup #63608 (review) the break-on-start is delivered only after runIfWaitingForDebugger, by which point started is already true and the existing if (hitBreakpoints === undefined || hitBreakpoints.length === 0) path already resumes it. The test only reaches the guarded state by manipulating internals to fabricate a state that's unreachable from probe mode in the real world.
  2. If in the future we implement attaching to a process that already has breakpoints, the previous code would resume if the breakpoint is not part of the probed ones, which is good. The changes here would actually make that hang without resuming, which would be undesirable.

Have you verified, without manipulating internals, that this actually fixes the flake in GitHub Actions? From the logs this looks like another reproduction of #63632 (comment), which predates probe mode - the interactive-mode tests are still hanging a lot from the same cause in CI, so I doubt changing probe mode fixes anything here.

aduh95 pushed a commit that referenced this pull request Jun 25, 2026
Keep the initial --inspect-brk pause held until probe breakpoints are
bound and probe mode explicitly releases the target. This prevents the
generic pause handler from resuming user code before probes are ready.
Refs: https://github.com/nodejs/node/actions/runs/26482141780/job/77981519238
Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
PR-URL: #63608
Refs: https://github.com/nodejs/node/actions/runs/26482141780/job/77981519238
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
aduh95 pushed a commit that referenced this pull request Jul 22, 2026
Keep the initial --inspect-brk pause held until probe breakpoints are
bound and probe mode explicitly releases the target. This prevents the
generic pause handler from resuming user code before probes are ready.
Refs: https://github.com/nodejs/node/actions/runs/26482141780/job/77981519238
Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
PR-URL: #63608
Refs: https://github.com/nodejs/node/actions/runs/26482141780/job/77981519238
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
aduh95 pushed a commit that referenced this pull request Jul 30, 2026
Keep the initial --inspect-brk pause held until probe breakpoints are
bound and probe mode explicitly releases the target. This prevents the
generic pause handler from resuming user code before probes are ready.
Refs: https://github.com/nodejs/node/actions/runs/26482141780/job/77981519238
Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
PR-URL: #63608
Refs: https://github.com/nodejs/node/actions/runs/26482141780/job/77981519238
Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

commit-queue-squashAdd this label to instruct the Commit Queue to squash all the PR commits into the first one.debuggerIssues and PRs related to the debugger subsystem.flaky-testIssues and PRs related to the tests with unstable failures on the CI.needs-ciPRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@trivikr@nodejs-github-bot@joyeecheung@watilde