Skip to content

report: skip unresponsive workers on process timeout - #66304

Open
trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:report-on-process-timeout-on-blocked-worker
Open

trivikr wants to merge 1 commit into
nodejs:mainfrom
trivikr:report-on-process-timeout-on-blocked-worker

Conversation

@trivikr

@trivikr trivikr commented Sep 25, 2026

Copy link
Copy Markdown
Member

Fixes: #66303

When --process-timeout expires, --report-on-process-timeout asked every Worker for a subreport and waited without a time limit. A Worker blocked in a synchronous native call never answers, so the watchdog force-exited the process before the report was written. That left a truncated, invalid JSON file, and the forced-exit message was glued onto the "Writing Node.js report to file" line.

For reports triggered by --process-timeout, wait at most two seconds for Worker subreports and leave out Worker threads that have not responded by then. The subreport state is now shared with the interrupt callbacks, so a Worker that answers late does not touch freed memory. Other report triggers are unchanged.


Assisted-by: claude:opus-5.5

@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run. labels Sep 25, 2026
When --process-timeout expires, --report-on-process-timeout asked
every Worker for a subreport and waited without a time limit. A Worker
blocked in a synchronous native call never answers, so the watchdog
force-exited the process before the report was written. That left a
truncated, invalid JSON file, and the forced-exit message was glued
onto the "Writing Node.js report to file" line.

For reports triggered by --process-timeout, wait at most two seconds
for Worker subreports and leave out Worker threads that have not
responded by then. The subreport state is now shared with the interrupt
callbacks, so a Worker that answers late does not touch freed memory.
Other report triggers are unchanged.

Signed-off-by: Trivikram Kamat <16024985+trivikr@users.noreply.github.com>
Assisted-by: claude:opus-5.5
@trivikr
trivikr force-pushed the report-on-process-timeout-on-blocked-worker branch from f5111e3 to 8c62c35 Compare September 25, 2026 22:12
@trivikr
trivikr requested a review from jasnell September 25, 2026 23:23
@trivikr

trivikr commented Sep 25, 2026

Copy link
Copy Markdown
Member Author

@jasnell I'd to force push after fixing a lint error.
Can you approve again?

@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 25, 2026
@codecov

codecov Bot commented Sep 26, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 68.96552% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.36%. Comparing base (4bd56b3) to head (8c62c35).
⚠️ Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/node_report.cc 79.16% 4 Missing and 1 partial ⚠️
src/node_mutex.h 0.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66304      +/-   ##
==========================================
- Coverage   90.37%   90.36%   -0.01%     
==========================================
  Files         790      790              
  Lines      274279   274284       +5     
  Branches    52506    52498       -8     
==========================================
- Hits       247870   247859      -11     
- Misses      16890    16903      +13     
- Partials     9519     9522       +3     
Files with missing lines Coverage Δ
src/node_watchdog.cc 79.82% <100.00%> (ø)
src/node_watchdog.h 44.44% <ø> (ø)
src/node_mutex.h 94.00% <0.00%> (-3.92%) ⬇️
src/node_report.cc 92.39% <79.16%> (-0.74%) ⬇️

... and 25 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 trivikr added author ready PRs with CI started, the required approvals, and no outstanding review comments. and removed request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. labels Sep 26, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. c++ Issues and PRs that require attention from people who are familiar with C++. lib / src Issues and PRs involving general changes in the lib/ or src/ directories. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

node --report-on-process-timeout writes truncated report when a Worker is blocked

3 participants