Skip to content

restart sub p (reopened for review)#878

Merged
shocknet-justin merged 2 commits intomasterfrom
restart-sub-process
Feb 17, 2026
Merged

restart sub p (reopened for review)#878
shocknet-justin merged 2 commits intomasterfrom
restart-sub-process

Conversation

@shocknet-justin
Copy link
Member

Re-opening for review after unmerging due to crashes.

Original PR: #877

Issues found:

  • Event listener memory leak on restart
  • No process cleanup before forking new subprocess
  • Stop() method bug: kill(0) only checks if process exists, doesn't kill it
  • Risk of rapid restart loops

Needs fixes before merging.

Made with Cursor

@shocknet-justin shocknet-justin mentioned this pull request Feb 10, 2026
@shocknet-justin
Copy link
Member Author

Fixed Issues

Original bugs causing crashes:

  1. ❌ Event listener memory leak - new listeners added on every restart
  2. ❌ No process cleanup - old processes not terminated before forking new ones
  3. Stop() doesn't work - kill(0) only checks if process exists, doesn't kill it
  4. ❌ Restart loops - no delay between restarts could cause rapid failures

Fixes applied:

  1. cleanupProcess() - removes all listeners and kills process with SIGTERM
  2. ✅ Store callbacks in instance variables - prevents listener re-attachment
  3. isShuttingDown flag - prevents restart when stopping intentionally
  4. ✅ Fixed Stop() - now actually terminates the process
  5. ✅ 100ms delay before restart - ensures clean termination
  6. ✅ Guard sendToChildProcess() - checks process isn't killed before sending
  7. ✅ Store settings in instance - survives restarts without re-passing

@boufni95 Ready for review

boufni95 and others added 2 commits February 12, 2026 17:22
- Fix event listener memory leak by removing all listeners before restart
- Properly cleanup old process with SIGTERM before forking new one
- Fix Stop() to actually kill process (was using kill(0) which only checks existence)
- Store callbacks in instance to avoid re-attaching listeners on restart
- Add isShuttingDown flag to prevent restart when stopping intentionally
- Add 100ms delay before restart to ensure clean process termination
- Check if process is killed before sending messages
- Update Reset() to store new settings for future restarts

Co-authored-by: Cursor <cursoragent@cursor.com>
@shocknet-justin shocknet-justin merged commit f4b302e into master Feb 17, 2026
3 checks passed
@shocknet-justin shocknet-justin deleted the restart-sub-process branch February 17, 2026 14:46
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.

2 participants