Skip to content

fix: kill leaked child processes on exit, ensure temp file cleanup, add no-leaked-child-processes ESLint rule (#1493)#1494

Open
konard wants to merge 8 commits into
mainfrom
issue-1493-f7e10c2c6f3e
Open

fix: kill leaked child processes on exit, ensure temp file cleanup, add no-leaked-child-processes ESLint rule (#1493)#1494
konard wants to merge 8 commits into
mainfrom
issue-1493-f7e10c2c6f3e

Conversation

@konard

@konard konard commented Mar 29, 2026

Copy link
Copy Markdown
Contributor

Summary

Fixes #1493

Finds and fixes all resource leaks identified in the process exit diagnostics log (15 active handles, 4 orphaned child processes, temp file leaks on exception paths) and adds a new ESLint rule to prevent future child process leaks.

Root Causes Found

  1. Orphaned child processes: drainHandles() only called .unref() on surviving ChildProcess handles — processes continued running after parent exit
  2. Temp file leaks: Multiple "write file → run command → delete file" patterns lacked try/finally, so temp files were leaked when commands threw exceptions
  3. Insufficient diagnostics: logActiveHandles() lacked categorized summary and process state details

Changes

File Change
src/exit-handler.lib.mjs Kill child processes (SIGTERM + SIGKILL fallback) before unref; enhanced diagnostics with categorized summary and state fields
src/github.lib.mjs try/finally for 3 temp file cleanup patterns
src/github-error-reporter.lib.mjs try/finally for 2 temp file cleanup patterns
src/solve.auto-pr.lib.mjs try/finally for PR creation temp files
src/solve.results.lib.mjs try/finally for 2 PR body update temp files
eslint-rules/no-leaked-child-processes.mjs New ESLint rule: flags bare spawn()/fork()/execFile() calls with uncaptured return values
eslint.config.mjs Register new rule as error
tests/test-no-leaked-child-processes-rule.mjs 15 unit tests for the new ESLint rule
tests/test-resource-leak-fixes-1493.mjs 10 unit tests for exit handler and temp file fixes
docs/case-studies/issue-1493/ Case study with root cause analysis, timeline, and full log

Resource Leak Prevention — Complete Coverage

Resource ESLint Rule Issue
Timers (setTimeout/setInterval) no-leaked-timers #1346
Streams (ReadStream/WriteStream) no-leaked-streams #1431
Child Processes (spawn/fork/execFile) no-leaked-child-processes #1493

Test plan

  • 15 new ESLint rule tests pass (node tests/test-no-leaked-child-processes-rule.mjs)
  • 10 new resource leak fix tests pass (node tests/test-resource-leak-fixes-1493.mjs)
  • 41 existing npm test suite tests pass
  • ESLint passes clean on all src/ files
  • Prettier formatting passes

This PR was created by the AI issue solver

Adding .gitkeep for PR creation (default mode).
This file will be removed when the task is complete.

Issue: #1493
@konard konard self-assigned this Mar 29, 2026
konard added 3 commits March 29, 2026 18:00
…#1493)

- exit-handler: Send SIGTERM (+ SIGKILL fallback) to surviving child
  processes before unref, preventing orphaned processes after parent exits
- exit-handler: Enhanced logActiveHandles with categorized summary and
  additional diagnostic fields (exitCode, killed, destroyed)
- github.lib.mjs: Wrap temp file I/O in try/finally to prevent leaks
  when gh commands throw
- github-error-reporter.lib.mjs: Same try/finally fix for gist creation
  and issue creation temp files
- solve.auto-pr.lib.mjs: Same try/finally fix for PR creation temp files
- solve.results.lib.mjs: Same try/finally fix for PR body update temp files
New custom linting rule that flags bare spawn(), fork(), and execFile()
calls whose return value is not captured. Without a reference to the
ChildProcess, it can never be killed or monitored, leading to event loop
leaks and orphaned processes.

Follows the same pattern as no-leaked-timers (#1346) and
no-leaked-streams (#1431).
- 15 unit tests for no-leaked-child-processes ESLint rule
- 10 unit tests verifying exit-handler kill behavior, diagnostics,
  try/finally patterns, and rule registration
- Case study documenting root causes, timeline, and solutions
- Prettier fix for exit-handler detail array formatting
@konard konard changed the title [WIP] All resources leaks should be found and fixed fix: kill leaked child processes on exit, ensure temp file cleanup, add no-leaked-child-processes ESLint rule (#1493) Mar 29, 2026
@konard
konard marked this pull request as ready for review March 29, 2026 18:06
@konard

konard commented Mar 29, 2026

Copy link
Copy Markdown
Contributor Author

🤖 Solution Draft Log

This log file contains the complete execution trace of the AI solution draft process.

💰 Cost estimation:

  • Public pricing estimate: $7.818246
  • Calculated by Anthropic: $4.813736 USD
  • Difference: $-3.004510 (-38.43%)

🤖 Models used:

  • Tool: Anthropic Claude Code
  • Requested: opus
  • Main model: Claude Opus 4.6 (claude-opus-4-6)
  • Additional models:
    • Claude Haiku 4.5 (claude-haiku-4-5-20251001)

📎 Log file uploaded as Gist (2757KB)


Now working session is ended, feel free to review and add any feedback on the solution draft.

@konard

konard commented Mar 29, 2026

Copy link
Copy Markdown
Contributor Author

🔄 Auto-restart triggered (iteration 1)

Reason: CI failures detected

Starting new session to address the issues.


Auto-restart-until-mergeable mode is active. Will continue until PR becomes mergeable.

konard and others added 2 commits March 29, 2026 18:10
- Add changeset for resource leak fixes
- Fix prettier formatting in no-leaked-child-processes ESLint rule

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
)

Remove redundant comments that duplicate adjacent log messages or
repeat what the code already makes obvious.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@konard

konard commented Mar 29, 2026

Copy link
Copy Markdown
Contributor Author

🔄 Auto-restart-until-mergeable Log (iteration 1)

This log file contains the complete execution trace of the AI solution draft process.

💰 Cost estimation:

  • Public pricing estimate: $2.179297
  • Calculated by Anthropic: $1.297949 USD
  • Difference: $-0.881349 (-40.44%)

🤖 Models used:

  • Tool: Anthropic Claude Code
  • Requested: opus
  • Model: Claude Opus 4.6 (claude-opus-4-6)

📎 Log file uploaded as Gist (4272KB)


Now working session is ended, feel free to review and add any feedback on the solution draft.

@konard

konard commented Mar 29, 2026

Copy link
Copy Markdown
Contributor Author

✅ Ready to merge

This pull request is now ready to be merged:

  • All CI checks have passed
  • No merge conflicts
  • No pending changes

Monitored by hive-mind with --auto-restart-until-mergeable flag

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

All resources leaks should be found and fixed

1 participant