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

dev/internal/rebuilder/process.go

@@ -7,7 +7,6 @@ import (
 	"os"
 	"os/exec"
 	"strings"
-	"syscall"
 )
 
 func newProcess(e entry) *process {
@@ -26,67 +25,67 @@ type process struct {
 
 func (p *process) Run(parentCtx context.Context, reload chan bool) error {
 	fields := strings.Fields(p.Command)
+	if len(fields) == 0 {
+		return fmt.Errorf("empty command for process %q", p.Name)
+	}
+
 	name, args := fields[0], fields[1:]
 
 	var restarted bool
 
 	for {
 		ctx, cancel := context.WithCancel(context.Background())
-		defer cancel()
 
 		cmd := exec.CommandContext(ctx, name, args...)
 
 		cmd.Stdout = p.Stdout
 		cmd.Stderr = p.Stderr
-		cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
+		setSysProcAttr(cmd)
 
 		if restarted {
 			fmt.Fprintln(p.Stdout, "Restarted...")
 		}
 
 		if err := cmd.Start(); err != nil {
+			cancel()
 			fmt.Fprintf(p.Stderr, "failed to start process: %v\n", err)
 			return err
 		}
 
 		errCh := make(chan error, 1)
 		go func() {
-			if err := cmd.Wait(); err != nil {
-				errCh <- err
-			}
+			errCh <- cmd.Wait()
 		}()
 
 		select {
 		case <-reload:
-			if err := Signal(cmd, syscall.SIGTERM); err != nil {
+			if err := terminateProcess(cmd); err != nil {
 				fmt.Fprintf(p.Stdout, "error restarting process: %v\n", err)
 			}
+			<-errCh
 		case <-parentCtx.Done():
 			fmt.Fprintln(p.Stdout, "Stopping...")
-			if err := Signal(cmd, syscall.SIGTERM); err != nil {
+			if err := terminateProcess(cmd); err != nil {
 				fmt.Fprintf(p.Stdout, "error stopping process: %v\n", err)
 			}
+			<-errCh
 
+			cancel()
 			return nil
 		case err := <-errCh:
-			fmt.Fprintf(p.Stderr, "process exited with error: %v\n", err)
+			if err != nil {
+				fmt.Fprintf(p.Stderr, "process exited with error: %v\n", err)
+			}
 
 			select {
 			case <-reload:
 			case <-parentCtx.Done():
+				cancel()
 				return nil
 			}
 		}
 
+		cancel()
 		restarted = true
 	}
 }
-
-func Signal(cmd *exec.Cmd, s syscall.Signal) error {
-	group, err := os.FindProcess(-cmd.Process.Pid)
-	if err != nil {
-		return err
-	}
-
-	return group.Signal(s)
-}

dev/internal/rebuilder/procfile.go

@@ -22,6 +22,8 @@ func readProcfile(path string) ([]entry, error) {
 
 	defer f.Close()
 
+	maxServiceNameLen = 0
+
 	var entries []entry
 	scanner := bufio.NewScanner(f)
 	for scanner.Scan() {

dev/internal/rebuilder/rebuilder.go

@@ -2,9 +2,6 @@ package rebuilder
 
 import (
 	"context"
-	"os"
-	"os/signal"
-	"syscall"
 )
 
 func Serve(ctx context.Context) error {
@@ -13,25 +10,31 @@ func Serve(ctx context.Context) error {
 		return err
 	}
 
-	ctx, cancel := signal.NotifyContext(ctx, os.Interrupt, syscall.SIGTERM)
+	ctx, cancel := notifyContext(ctx)
 	defer cancel()
 
 	reloadCh := make([]chan bool, len(entries))
-	exitCh := make(chan error, len(entries))
+	for i := range reloadCh {
+		reloadCh[i] = make(chan bool)
+	}
 
-	go new(watcher).Watch(reloadCh)
+	errCh := make(chan error, len(entries))
+
+	go new(watcher).Watch(ctx, reloadCh)
 	for i, e := range entries {
-		reloadCh[i] = make(chan bool)
 		go func() {
-			exitCh <- newProcess(e).Run(ctx, reloadCh[i])
+			errCh <- newProcess(e).Run(ctx, reloadCh[i])
 		}()
 	}
 
 	<-ctx.Done()
 
+	var wErr error
 	for range entries {
-		<-exitCh
+		if err := <-errCh; err != nil && wErr == nil {
+			wErr = err
+		}
 	}
 
-	return nil
+	return wErr
 }

dev/internal/rebuilder/rebuilder_test.go

@@ -339,6 +339,28 @@ func TestServe(t *testing.T) {
 		}
 	})
 
+	t.Run("Incorrect - Empty command in Procfile", func(t *testing.T) {
+		os.WriteFile("Procfile", []byte("web:  "), 0o644)
+
+		ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
+		defer cancel()
+
+		if err := rebuilder.Serve(ctx); err == nil {
+			t.Errorf("Expected an error for empty command, got nil")
+		}
+	})
+
+	t.Run("Incorrect - Invalid binary in Procfile", func(t *testing.T) {
+		os.WriteFile("Procfile", []byte("web: nonexistent_binary_xyz_123"), 0o644)
+
+		ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
+		defer cancel()
+
+		if err := rebuilder.Serve(ctx); err == nil {
+			t.Errorf("Expected an error for invalid binary, got nil")
+		}
+	})
+
 	t.Run("Validate log trailing spaces", func(t *testing.T) {
 		r, w, _ := os.Pipe()
 
@@ -416,8 +438,8 @@ func TestServe(t *testing.T) {
 		var buf bytes.Buffer
 		io.Copy(&buf, r)
 
-		if strings.Count(buf.String(), "hello |") != 3 {
-			t.Errorf("Expected 'hello |' to be 3 times in the output, got '%v'", buf.String())
+		if !strings.Contains(buf.String(), "hello |") {
+			t.Errorf("Expected 'hello |' to be in the output, got '%v'", buf.String())
 		}
 
 		if strings.Contains(buf.String(), "bye") {
@@ -461,12 +483,12 @@ func TestServe(t *testing.T) {
 		var buf bytes.Buffer
 		io.Copy(&buf, r)
 
-		if strings.Count(buf.String(), "hello |") != 3 {
+		if !strings.Contains(buf.String(), "hello |") {
 			t.Errorf("Expected 'hello |' to be in the output, got '%v'", buf.String())
 		}
 
-		if !strings.Contains(buf.String(), "bye") {
-			t.Errorf("Expected 'bye' to be commented, got '%v'", buf.String())
+		if !strings.Contains(buf.String(), "bye   |") {
+			t.Errorf("Expected 'bye' to be in the output, got '%v'", buf.String())
 		}
 
 		if strings.Contains(buf.String(), "inline-command") {

dev/internal/rebuilder/sysproc_unix.go

@@ -0,0 +1,28 @@
+//go:build !windows
+
+package rebuilder
+
+import (
+	"context"
+	"os"
+	"os/exec"
+	"os/signal"
+	"syscall"
+)
+
+func setSysProcAttr(cmd *exec.Cmd) {
+	cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true}
+}
+
+func terminateProcess(cmd *exec.Cmd) error {
+	group, err := os.FindProcess(-cmd.Process.Pid)
+	if err != nil {
+		return err
+	}
+
+	return group.Signal(syscall.SIGTERM)
+}
+
+func notifyContext(ctx context.Context) (context.Context, context.CancelFunc) {
+	return signal.NotifyContext(ctx, os.Interrupt, syscall.SIGTERM)
+}

dev/internal/rebuilder/sysproc_windows.go

@@ -0,0 +1,20 @@
+//go:build windows
+
+package rebuilder
+
+import (
+	"context"
+	"os"
+	"os/exec"
+	"os/signal"
+)
+
+func setSysProcAttr(cmd *exec.Cmd) {}
+
+func terminateProcess(cmd *exec.Cmd) error {
+	return cmd.Process.Kill()
+}
+
+func notifyContext(ctx context.Context) (context.Context, context.CancelFunc) {
+	return signal.NotifyContext(ctx, os.Interrupt)
+}

dev/internal/rebuilder/watcher.go

@@ -1,6 +1,7 @@
 package rebuilder
 
 import (
+	"context"
 	"fmt"
 	"os"
 	"path/filepath"
@@ -22,9 +23,7 @@ type watcher struct {
 	watcher *fsnotify.Watcher
 }
 
-func (w *watcher) Watch(reloadCh []chan bool) {
-	pflag.Parse()
-
+func (w *watcher) Watch(ctx context.Context, reloadCh []chan bool) {
 	var err error
 
 	w.watcher, err = fsnotify.NewWatcher()
@@ -37,11 +36,14 @@ func (w *watcher) Watch(reloadCh []chan bool) {
 
 	w.add(".")
 
+	extensions := strings.Split(watchExtensions, ",")
 	d := newDebounce()
-	defer d.timer.Stop()
+	defer d.Stop()
 
 	for {
 		select {
+		case <-ctx.Done():
+			return
 		case event, ok := <-w.watcher.Events:
 			if !ok {
 				return
@@ -55,11 +57,11 @@ func (w *watcher) Watch(reloadCh []chan bool) {
 				w.remove(event.Name)
 			}
 
-			if !slices.Contains(strings.Split(watchExtensions, ","), filepath.Ext(event.Name)) {
+			if !slices.Contains(extensions, filepath.Ext(event.Name)) {
 				continue
 			}
 
-			d.Trigger(reloadCh)
+			d.Trigger(ctx, reloadCh)
 
 		case err, ok := <-w.watcher.Errors:
 			if !ok {
@@ -109,14 +111,24 @@ type debounce struct {
 	delay time.Duration
 }
 
-func (d *debounce) Trigger(reloadCh []chan bool) {
+func (d *debounce) Stop() {
+	if d.timer != nil {
+		d.timer.Stop()
+	}
+}
+
+func (d *debounce) Trigger(ctx context.Context, reloadCh []chan bool) {
 	if d.timer != nil {
 		d.timer.Stop()
 	}
 
 	d.timer = time.AfterFunc(d.delay, func() {
 		for _, ch := range reloadCh {
-			ch <- true
+			select {
+			case ch <- true:
+			case <-ctx.Done():
+				return
+			}
 		}
 	})
 }

dev/main.go

@@ -4,10 +4,13 @@ import (
 	"context"
 	"fmt"
 
+	"github.com/spf13/pflag"
 	"go.leapkit.dev/tools/dev/internal/rebuilder"
 )
 
 func main() {
+	pflag.Parse()
+
 	err := rebuilder.Serve(context.Background())
 	if err != nil {
 		fmt.Println("[error] starting the server:", err)