Skip to content

fix(rebuilder): fix rebuilder for windows and goroutine leaks and process lifecycle bugs - #13

Merged
paganotoni merged 3 commits into
mainfrom
improving-rebuilder
Mar 13, 2026
Merged

fix(rebuilder): fix rebuilder for windows and goroutine leaks and process lifecycle bugs#13
paganotoni merged 3 commits into
mainfrom
improving-rebuilder

Conversation

@tatang26

Copy link
Copy Markdown
Contributor

Summary

  • Fix critical goroutine and file-descriptor leaks in the file watcher by threading context.Context through Watch()
  • Fix debounce timer blocking after shutdown with context-aware channel sends
  • Extract platform-specific process handling (setSysProcAttr, terminateProcess, notifyContext) into sysproc_unix.go / sysproc_windows.go
  • Fix process lifecycle: explicit context cancellation, post-terminate wait, empty command guard, error propagation from Serve()
  • Fix maxServiceNameLen global accumulation across multiple Serve() calls
  • Move pflag.Parse() from goroutine to main() for deterministic parsing

Bugs Fixed

Severity Issue
Critical Watcher goroutine never stops — no context/done channel in Watch()
Critical Debounce timer callback blocks on unbuffered channel after shutdown
Critical fsnotify file descriptor never closed (consequence of watcher leak)
High defer cancel() inside for loop accumulates contexts per restart
High Clean process exit invisible — errCh only received errors, not nil
High reloadCh data race — channels initialized after watcher started
Medium pflag.Parse() called inside goroutine
Medium No wait after terminateProcess — process not fully reaped before restart
Medium Panic on empty command in Procfile
Medium maxServiceNameLen accumulates across test runs (-count>1)

Testing

  • 16 tests pass with -race -count=3
  • Coverage: 87.3%
  • All tests go through the public Serve() API

tatang26 added 3 commits March 13, 2026 10:51
Extract setSysProcAttr, terminateProcess, and notifyContext into
sysproc_unix.go and sysproc_windows.go to isolate platform-specific
syscall usage behind a common interface. This enables clean builds
on both platforms without build tag complexity in business logic.
… lifecycle

- Thread context.Context through Watch() so watcher goroutine exits on
  shutdown, preventing goroutine and fsnotify file-descriptor leaks
- Use context-aware sends in debounce timer callback to prevent blocking
  after shutdown; add nil-safe debounce.Stop() to avoid panic
- Move channel initialization before watcher start to eliminate data race
- Add explicit cancel() calls and post-terminate errCh drain to ensure
  child processes are fully waited on before restart
- Guard against empty command panic in startProcess
- Collect and return first non-nil error from Serve() via exitCh
- Move pflag.Parse() from goroutine to main() for deterministic parsing
- Update test expectations and add error-case subtests for empty command
  and invalid binary scenarios
The package-level maxServiceNameLen accumulated across multiple
Serve() calls and test runs (-count>1), causing inconsistent
log formatting. Reset to 0 at the start of readProcfile().
@tatang26 tatang26 changed the title fix(rebuilder): fix goroutine leaks, resource leaks, and process lifecycle bugs fix(rebuilder): fix rebuilder for windows and goroutine leaks and process lifecycle bugs Mar 13, 2026
@paganotoni
paganotoni merged commit d6b2899 into main Mar 13, 2026
1 check passed
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