From 77f4c9b092d0cb7e4fbb3b22f0970215d749ac9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9?= Date: Wed, 28 Jan 2026 08:50:09 +0100 Subject: [PATCH 1/8] feat(prefork): add WatchMaster and callback support for child process management --- prefork/prefork.go | 115 +++++++++++++++++--- prefork/prefork_test.go | 232 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 335 insertions(+), 12 deletions(-) diff --git a/prefork/prefork.go b/prefork/prefork.go index 9541a8fede..dc133fc278 100644 --- a/prefork/prefork.go +++ b/prefork/prefork.go @@ -8,6 +8,7 @@ import ( "os" "os/exec" "runtime" + "time" "github.com/valyala/fasthttp" "github.com/valyala/fasthttp/reuseport" @@ -16,6 +17,8 @@ import ( const ( preforkChildEnvVariable = "FASTHTTP_PREFORK_CHILD" defaultNetwork = "tcp4" + windowsOS = "windows" + watchInterval = 500 * time.Millisecond ) var ( @@ -68,6 +71,30 @@ type Prefork struct { // // It's disabled by default Reuseport bool + + // WatchMaster enables monitoring of the master process. + // If enabled, child processes will automatically exit when the master process dies. + // + // It's disabled by default + WatchMaster bool + + // OnChildSpawn is called in the master process whenever a new child process is spawned. + // It receives the PID of the newly spawned child process. + // + // If this callback returns an error, the prefork operation will be aborted. + OnChildSpawn func(pid int) error + + // OnMasterReady is called in the master process after all child processes have been spawned. + // It receives a slice of all child process PIDs. + // + // If this callback returns an error, the prefork operation will be aborted. + OnMasterReady func(childPIDs []int) error + + // OnChildRecover is called in the master process when a child process is restarted + // after a crash. It receives the PID of the newly recovered child process. + // + // This callback is non-blocking and its error return value is ignored. + OnChildRecover func(pid int) error } // IsChild checks if the current thread/process is a child. @@ -101,6 +128,11 @@ func (p *Prefork) listen(addr string) (net.Listener, error) { p.Network = defaultNetwork } + // Start watching master process if enabled + if p.WatchMaster { + go watchMaster() + } + if p.Reuseport { return reuseport.Listen(p.Network, addr) } @@ -172,7 +204,8 @@ func (p *Prefork) prefork(addr string) (err error) { goMaxProcs := runtime.GOMAXPROCS(0) sigCh := make(chan procSig, goMaxProcs) - childProcs := make(map[int]*exec.Cmd) + // Pre-allocate map with expected capacity for better performance + childProcs := make(map[int]*exec.Cmd, goMaxProcs) defer func() { for _, proc := range childProcs { @@ -180,17 +213,40 @@ func (p *Prefork) prefork(addr string) (err error) { } }() + // Pre-allocate slice with expected capacity for OnMasterReady callback + childPIDs := make([]int, 0, goMaxProcs) + for i := 0; i < goMaxProcs; i++ { - var cmd *exec.Cmd - if cmd, err = p.doCommand(); err != nil { + cmd, err := p.doCommand() + if err != nil { p.logger().Printf("failed to start a child prefork process, error: %v\n", err) return err } - childProcs[cmd.Process.Pid] = cmd - go func() { - sigCh <- procSig{pid: cmd.Process.Pid, err: cmd.Wait()} - }() + pid := cmd.Process.Pid + childProcs[pid] = cmd + childPIDs = append(childPIDs, pid) + + // Call OnChildSpawn callback + if p.OnChildSpawn != nil { + if err = p.OnChildSpawn(pid); err != nil { + p.logger().Printf("OnChildSpawn callback failed for PID %d: %v\n", pid, err) + return err + } + } + + // Pass cmd to goroutine to avoid closure capture bug + go func(c *exec.Cmd, pid int) { + sigCh <- procSig{pid: pid, err: c.Wait()} + }(cmd, pid) + } + + // Call OnMasterReady callback after all children are spawned + if p.OnMasterReady != nil { + if err = p.OnMasterReady(childPIDs); err != nil { + p.logger().Printf("OnMasterReady callback failed: %v\n", err) + return err + } } var exitedProcs int @@ -210,13 +266,22 @@ func (p *Prefork) prefork(addr string) (err error) { } var cmd *exec.Cmd - if cmd, err = p.doCommand(); err != nil { + cmd, err = p.doCommand() + if err != nil { break } - childProcs[cmd.Process.Pid] = cmd - go func() { - sigCh <- procSig{pid: cmd.Process.Pid, err: cmd.Wait()} - }() + pid := cmd.Process.Pid + childProcs[pid] = cmd + + // Call OnChildRecover callback (non-blocking, error ignored) + if p.OnChildRecover != nil { + _ = p.OnChildRecover(pid) + } + + // Pass cmd to goroutine to avoid closure capture bug + go func(c *exec.Cmd, pid int) { + sigCh <- procSig{pid: pid, err: c.Wait()} + }(cmd, pid) } return err @@ -273,3 +338,29 @@ func (p *Prefork) ListenAndServeTLSEmbed(addr string, certData, keyData []byte) return p.prefork(addr) } + +// watchMaster monitors the master process and exits the child process if the master dies. +// This ensures that orphaned child processes are properly cleaned up. +// +// On Windows: Uses os.FindProcess and Wait() to detect master process exit. +// On Unix/Linux: Periodically checks if parent PID is 1 (init process), indicating orphan status. +func watchMaster() { + // Windows implementation + if runtime.GOOS == windowsOS { + // Find parent process + p, err := os.FindProcess(os.Getppid()) + if err == nil { + _, _ = p.Wait() // Wait for parent to exit + } + os.Exit(1) //nolint:revive // Exiting child process is intentional + } + + // Unix/Linux implementation + // If parent PID becomes 1 (init), it means the master process has died + // and this child process has been adopted by init + for range time.NewTicker(watchInterval).C { + if os.Getppid() == 1 { + os.Exit(1) //nolint:revive // Exiting child process is intentional + } + } +} diff --git a/prefork/prefork_test.go b/prefork/prefork_test.go index 8236e124e4..5c01906ca2 100644 --- a/prefork/prefork_test.go +++ b/prefork/prefork_test.go @@ -224,3 +224,235 @@ func Test_ListenAndServeTLSEmbed(t *testing.T) { t.Error("Prefork.ln is nil") } } + +func Test_Prefork_Logger(t *testing.T) { + t.Parallel() + + s := &fasthttp.Server{} + p := New(s) + + // Test default logger + logger := p.logger() + if logger == nil { + t.Error("Default logger should not be nil") + } + + // Test custom logger + customLogger := &testLogger{} + p.Logger = customLogger + if p.logger() != customLogger { + t.Error("Custom logger should be returned") + } +} + +type testLogger struct { + messages []string +} + +func (l *testLogger) Printf(format string, args ...any) { + l.messages = append(l.messages, fmt.Sprintf(format, args...)) +} + +func Test_Prefork_WatchMaster(t *testing.T) { + t.Parallel() + + p := &Prefork{ + Reuseport: true, + WatchMaster: true, + } + + // Verify WatchMaster is set + if !p.WatchMaster { + t.Error("WatchMaster should be true") + } +} + +func Test_Prefork_Callbacks_NotNil(t *testing.T) { + t.Parallel() + + var spawnCalled bool + var readyCalled bool + var recoverCalled bool + + p := &Prefork{ + OnChildSpawn: func(pid int) error { + spawnCalled = true + return nil + }, + OnMasterReady: func(childPIDs []int) error { + readyCalled = true + return nil + }, + OnChildRecover: func(pid int) error { + recoverCalled = true + return nil + }, + } + + // Test that callbacks are set + if p.OnChildSpawn == nil { + t.Error("OnChildSpawn should not be nil") + } + if p.OnMasterReady == nil { + t.Error("OnMasterReady should not be nil") + } + if p.OnChildRecover == nil { + t.Error("OnChildRecover should not be nil") + } + + // Test that callbacks can be called + _ = p.OnChildSpawn(1234) + _ = p.OnMasterReady([]int{1234, 5678}) + _ = p.OnChildRecover(9999) + + if !spawnCalled { + t.Error("OnChildSpawn was not called") + } + if !readyCalled { + t.Error("OnMasterReady was not called") + } + if !recoverCalled { + t.Error("OnChildRecover was not called") + } +} + +func Test_Prefork_Callbacks_Nil(t *testing.T) { + t.Parallel() + + // Test that nil callbacks don't panic when checked + p := &Prefork{} + + if p.OnChildSpawn != nil { + t.Error("OnChildSpawn should be nil by default") + } + if p.OnMasterReady != nil { + t.Error("OnMasterReady should be nil by default") + } + if p.OnChildRecover != nil { + t.Error("OnChildRecover should be nil by default") + } +} + +func Test_Prefork_RecoverThreshold(t *testing.T) { + t.Parallel() + + s := &fasthttp.Server{} + p := New(s) + + // Default should be GOMAXPROCS/2 + expected := runtime.GOMAXPROCS(0) / 2 + if p.RecoverThreshold != expected { + t.Errorf("RecoverThreshold == %d, want %d", p.RecoverThreshold, expected) + } + + // Test custom threshold + p.RecoverThreshold = 10 + if p.RecoverThreshold != 10 { + t.Errorf("RecoverThreshold == %d, want %d", p.RecoverThreshold, 10) + } +} + +func Test_ErrOverRecovery(t *testing.T) { + t.Parallel() + + if ErrOverRecovery == nil { + t.Error("ErrOverRecovery should not be nil") + } + if ErrOverRecovery.Error() != "exceeding the value of RecoverThreshold" { + t.Errorf("ErrOverRecovery message incorrect: %s", ErrOverRecovery.Error()) + } +} + +func Test_ErrOnlyReuseportOnWindows(t *testing.T) { + t.Parallel() + + if ErrOnlyReuseportOnWindows == nil { + t.Error("ErrOnlyReuseportOnWindows should not be nil") + } + if ErrOnlyReuseportOnWindows.Error() != "windows only supports Reuseport = true" { + t.Errorf("ErrOnlyReuseportOnWindows message incorrect: %s", ErrOnlyReuseportOnWindows.Error()) + } +} + +func Test_Listen_WithWatchMaster(t *testing.T) { + // This test can't run parallel as it modifies env. + + setUp() + defer tearDown() + + p := &Prefork{ + Reuseport: true, + WatchMaster: true, + } + addr := getAddr() + + ln, err := p.listen(addr) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + defer ln.Close() + + // Verify listener was created + if ln == nil { + t.Error("Listener should not be nil") + } +} + +func Test_OnChildSpawn_Error(t *testing.T) { + t.Parallel() + + errExpected := fmt.Errorf("spawn callback error") + p := &Prefork{ + OnChildSpawn: func(pid int) error { + return errExpected + }, + } + + // Test that error is returned correctly + err := p.OnChildSpawn(1234) + if err != errExpected { + t.Errorf("OnChildSpawn error == %v, want %v", err, errExpected) + } +} + +func Test_OnMasterReady_Error(t *testing.T) { + t.Parallel() + + errExpected := fmt.Errorf("master ready callback error") + p := &Prefork{ + OnMasterReady: func(childPIDs []int) error { + return errExpected + }, + } + + // Test that error is returned correctly + err := p.OnMasterReady([]int{1, 2, 3}) + if err != errExpected { + t.Errorf("OnMasterReady error == %v, want %v", err, errExpected) + } +} + +func Test_OnMasterReady_ReceivesPIDs(t *testing.T) { + t.Parallel() + + var receivedPIDs []int + p := &Prefork{ + OnMasterReady: func(childPIDs []int) error { + receivedPIDs = childPIDs + return nil + }, + } + + expectedPIDs := []int{100, 200, 300} + _ = p.OnMasterReady(expectedPIDs) + + if len(receivedPIDs) != len(expectedPIDs) { + t.Errorf("Received %d PIDs, want %d", len(receivedPIDs), len(expectedPIDs)) + } + + for i, pid := range expectedPIDs { + if receivedPIDs[i] != pid { + t.Errorf("PID[%d] == %d, want %d", i, receivedPIDs[i], pid) + } + } +} From 81e8463a276d0499cf8175cc60e3f95c0d4e3140 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9?= Date: Wed, 28 Jan 2026 13:09:03 +0100 Subject: [PATCH 2/8] feat(prefork): add CommandProducer for customizable child process commands --- prefork/prefork.go | 13 ++++++++++++ prefork/prefork_test.go | 46 +++++++++++++++++++++++++++++++++++++++++ 2 files changed, 59 insertions(+) diff --git a/prefork/prefork.go b/prefork/prefork.go index dc133fc278..af6f7e8265 100644 --- a/prefork/prefork.go +++ b/prefork/prefork.go @@ -95,6 +95,13 @@ type Prefork struct { // // This callback is non-blocking and its error return value is ignored. OnChildRecover func(pid int) error + + // CommandProducer is called to create child process commands. + // If nil, the default implementation using os.Args is used. + // This is useful for testing or customizing child process behavior. + // + // The function receives the files to be passed as ExtraFiles to the child process. + CommandProducer func(files []*os.File) (*exec.Cmd, error) } // IsChild checks if the current thread/process is a child. @@ -168,6 +175,12 @@ func (p *Prefork) setTCPListenerFiles(addr string) error { } func (p *Prefork) doCommand() (*exec.Cmd, error) { + // Use custom CommandProducer if provided + if p.CommandProducer != nil { + return p.CommandProducer(p.files) + } + + // Default implementation // #nosec G204 cmd := exec.Command(os.Args[0], os.Args[1:]...) cmd.Stdout = os.Stdout diff --git a/prefork/prefork_test.go b/prefork/prefork_test.go index 5c01906ca2..bab9007357 100644 --- a/prefork/prefork_test.go +++ b/prefork/prefork_test.go @@ -5,6 +5,7 @@ import ( "math/rand" "net" "os" + "os/exec" "reflect" "runtime" "testing" @@ -456,3 +457,48 @@ func Test_OnMasterReady_ReceivesPIDs(t *testing.T) { } } } + +func Test_CommandProducer(t *testing.T) { + t.Parallel() + + var producerCalled bool + p := &Prefork{ + CommandProducer: func(files []*os.File) (*exec.Cmd, error) { + producerCalled = true + // Return a simple command that exits quickly + cmd := exec.Command("go", "version") + cmd.ExtraFiles = files + err := cmd.Start() + return cmd, err + }, + } + + // Verify CommandProducer is set + if p.CommandProducer == nil { + t.Error("CommandProducer should not be nil") + } + + // Call doCommand and verify our producer was used + cmd, err := p.doCommand() + if err != nil { + t.Fatalf("doCommand failed: %v", err) + } + + // Wait for the command to finish + _ = cmd.Wait() + + if !producerCalled { + t.Error("CommandProducer was not called") + } +} + +func Test_CommandProducer_Nil_UsesDefault(t *testing.T) { + t.Parallel() + + p := &Prefork{} + + // Verify default CommandProducer is nil + if p.CommandProducer != nil { + t.Error("CommandProducer should be nil by default") + } +} From 94d5cc6b65288ee297f7c1b245a903c4dae45483 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9?= Date: Fri, 6 Feb 2026 09:37:43 +0100 Subject: [PATCH 3/8] refactor(prefork): improve comments and parameter order in ListenAndServeTLS --- prefork/prefork.go | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/prefork/prefork.go b/prefork/prefork.go index af6f7e8265..d4f6379dfc 100644 --- a/prefork/prefork.go +++ b/prefork/prefork.go @@ -93,14 +93,15 @@ type Prefork struct { // OnChildRecover is called in the master process when a child process is restarted // after a crash. It receives the PID of the newly recovered child process. // - // This callback is non-blocking and its error return value is ignored. + // The callback's error return value is ignored. OnChildRecover func(pid int) error // CommandProducer is called to create child process commands. // If nil, the default implementation using os.Args is used. - // This is useful for testing or customizing child process behavior. + // This can be used for testing or customizing child process behavior. // - // The function receives the files to be passed as ExtraFiles to the child process. + // The function receives the files to be passed as ExtraFiles to the child process + // and must return a started command. CommandProducer func(files []*os.File) (*exec.Cmd, error) } @@ -319,7 +320,7 @@ func (p *Prefork) ListenAndServe(addr string) error { // ListenAndServeTLS serves HTTPS requests from the given TCP addr. // // certFile and keyFile are paths to TLS certificate and key files. -func (p *Prefork) ListenAndServeTLS(addr, certKey, certFile string) error { +func (p *Prefork) ListenAndServeTLS(addr, certFile, keyFile string) error { if IsChild() { ln, err := p.listen(addr) if err != nil { @@ -328,7 +329,7 @@ func (p *Prefork) ListenAndServeTLS(addr, certKey, certFile string) error { p.ln = ln - return p.ServeTLSFunc(ln, certFile, certKey) + return p.ServeTLSFunc(ln, certFile, keyFile) } return p.prefork(addr) @@ -371,7 +372,10 @@ func watchMaster() { // Unix/Linux implementation // If parent PID becomes 1 (init), it means the master process has died // and this child process has been adopted by init - for range time.NewTicker(watchInterval).C { + ticker := time.NewTicker(watchInterval) + defer ticker.Stop() + + for range ticker.C { if os.Getppid() == 1 { os.Exit(1) //nolint:revive // Exiting child process is intentional } From 72a6256ff46e033317208048d0fdbf68da399a5f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9?= Date: Fri, 6 Feb 2026 09:44:26 +0100 Subject: [PATCH 4/8] refactor(prefork): enhance logging message and clarify OnChildRecover callback comment --- prefork/prefork.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/prefork/prefork.go b/prefork/prefork.go index d4f6379dfc..6c1f9222e3 100644 --- a/prefork/prefork.go +++ b/prefork/prefork.go @@ -274,7 +274,7 @@ func (p *Prefork) prefork(addr string) (err error) { if exitedProcs > p.RecoverThreshold { p.logger().Printf("child prefork processes exit too many times, "+ "which exceeds the value of RecoverThreshold(%d), "+ - "exiting the master process.\n", exitedProcs) + "exiting the master process.\n", p.RecoverThreshold) err = ErrOverRecovery break } @@ -287,7 +287,7 @@ func (p *Prefork) prefork(addr string) (err error) { pid := cmd.Process.Pid childProcs[pid] = cmd - // Call OnChildRecover callback (non-blocking, error ignored) + // Call OnChildRecover callback and ignore its returned error. if p.OnChildRecover != nil { _ = p.OnChildRecover(pid) } From 768b8d66f1d2ef31a59ad10b4342d3363c07110c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9?= Date: Sat, 11 Apr 2026 19:25:35 +0200 Subject: [PATCH 5/8] fix(prefork): add Windows support to watchMaster On Windows, os.Getppid() returns a static PID that doesn't change when the parent exits (no reparenting). Use FindProcess+Wait instead, which correctly detects parent exit. Also document why masterPID comparison works for Docker containers (master PID 1 case). Co-Authored-By: Claude Opus 4.6 (1M context) --- prefork/prefork.go | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/prefork/prefork.go b/prefork/prefork.go index 85a8a86012..c29e532c9e 100644 --- a/prefork/prefork.go +++ b/prefork/prefork.go @@ -130,6 +130,22 @@ func (p *Prefork) logger() Logger { } func (p *Prefork) watchMaster(masterPID int) { + if runtime.GOOS == "windows" { + // On Windows, os.Getppid() returns a static PID that doesn't change + // when the parent exits (no reparenting). Use FindProcess+Wait instead. + proc, err := os.FindProcess(masterPID) + if err == nil { + _, _ = proc.Wait() + } + p.logger().Printf("master process died\n") + p.OnMasterDeath() + return + } + + // Unix/Linux/macOS: When the master exits, the OS reparents the child + // to another process, causing Getppid() to change. Comparing against + // the original masterPID (instead of hardcoding 1) ensures this works + // correctly when the master itself is PID 1 (e.g. in Docker containers). ticker := time.NewTicker(500 * time.Millisecond) defer ticker.Stop() From 2802b1a6a28c1a9e71fbe250894f8853d6182ceb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9?= Date: Sat, 11 Apr 2026 19:51:40 +0200 Subject: [PATCH 6/8] refactor(prefork): extract listenAsChild to eliminate DRY violation The three ListenAndServe* methods had identical child setup code (listen, set ln, watch master). Extract to listenAsChild() for cleaner code. Also add comment for the magic file descriptor number 3. Co-Authored-By: Claude Opus 4.6 (1M context) --- prefork/prefork.go | 45 +++++++++++++++++++++------------------------ 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/prefork/prefork.go b/prefork/prefork.go index c29e532c9e..31eb06b0ad 100644 --- a/prefork/prefork.go +++ b/prefork/prefork.go @@ -169,9 +169,27 @@ func (p *Prefork) listen(addr string) (net.Listener, error) { return reuseport.Listen(p.Network, addr) } + // File descriptor 3 is the first ExtraFiles entry passed by the master process. return net.FileListener(os.NewFile(3, "")) } +// listenAsChild performs the common child process setup: creates the listener +// and starts watching the master process if OnMasterDeath is configured. +func (p *Prefork) listenAsChild(addr string) (net.Listener, error) { + ln, err := p.listen(addr) + if err != nil { + return nil, err + } + + p.ln = ln + + if p.OnMasterDeath != nil { + go p.watchMaster(os.Getppid()) + } + + return ln, nil +} + func (p *Prefork) setTCPListenerFiles(addr string) error { if p.Network == "" { p.Network = defaultNetwork @@ -336,17 +354,10 @@ func (p *Prefork) prefork(addr string) (err error) { // ListenAndServe serves HTTP requests from the given TCP addr. func (p *Prefork) ListenAndServe(addr string) error { if IsChild() { - ln, err := p.listen(addr) + ln, err := p.listenAsChild(addr) if err != nil { return err } - - p.ln = ln - - if p.OnMasterDeath != nil { - go p.watchMaster(os.Getppid()) - } - return p.ServeFunc(ln) } @@ -358,17 +369,10 @@ func (p *Prefork) ListenAndServe(addr string) error { // certFile and certKey are paths to TLS certificate and key files. func (p *Prefork) ListenAndServeTLS(addr, certFile, certKey string) error { if IsChild() { - ln, err := p.listen(addr) + ln, err := p.listenAsChild(addr) if err != nil { return err } - - p.ln = ln - - if p.OnMasterDeath != nil { - go p.watchMaster(os.Getppid()) - } - return p.ServeTLSFunc(ln, certFile, certKey) } @@ -380,17 +384,10 @@ func (p *Prefork) ListenAndServeTLS(addr, certFile, certKey string) error { // certData and keyData must contain valid TLS certificate and key data. func (p *Prefork) ListenAndServeTLSEmbed(addr string, certData, keyData []byte) error { if IsChild() { - ln, err := p.listen(addr) + ln, err := p.listenAsChild(addr) if err != nil { return err } - - p.ln = ln - - if p.OnMasterDeath != nil { - go p.watchMaster(os.Getppid()) - } - return p.ServeTLSEmbedFunc(ln, certData, keyData) } From c1055ce62c371debc03a30ac318d1ab06680cf54 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9?= Date: Sat, 11 Apr 2026 20:19:36 +0200 Subject: [PATCH 7/8] fix(prefork): restore upstream ListenAndServeTLS parameter order Keep upstream's (addr, certKey, certFile) signature to avoid breaking callers. Fix the doc comment to match the actual parameter order instead. Co-Authored-By: Claude Opus 4.6 (1M context) --- prefork/prefork.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/prefork/prefork.go b/prefork/prefork.go index 31eb06b0ad..2aad3ff94f 100644 --- a/prefork/prefork.go +++ b/prefork/prefork.go @@ -366,8 +366,8 @@ func (p *Prefork) ListenAndServe(addr string) error { // ListenAndServeTLS serves HTTPS requests from the given TCP addr. // -// certFile and certKey are paths to TLS certificate and key files. -func (p *Prefork) ListenAndServeTLS(addr, certFile, certKey string) error { +// certKey and certFile are paths to TLS key and certificate files. +func (p *Prefork) ListenAndServeTLS(addr, certKey, certFile string) error { if IsChild() { ln, err := p.listenAsChild(addr) if err != nil { From be2ef67270a32712e777078f8531b2dc373af0e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9?= Date: Mon, 13 Apr 2026 08:38:25 +0200 Subject: [PATCH 8/8] fix(prefork): address lint errors and review feedback Lint fixes: - Remove unused Reuseport field write in test (govet/unusedwrite) - Replace fmt.Errorf with errors.New for static errors (perfsprint) Review feedback (Copilot): - Validate CommandProducer returns a started command (nil/Process check) - Clarify ListenAndServeTLS doc: parameter order and internal forwarding - Use hermetic test binary re-exec instead of external 'go' binary - Rename misleading test to reflect what it actually asserts Co-Authored-By: Claude Opus 4.6 (1M context) --- prefork/prefork.go | 15 +++++++++++++-- prefork/prefork_test.go | 22 ++++++++-------------- 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/prefork/prefork.go b/prefork/prefork.go index 2aad3ff94f..4dc00f6b47 100644 --- a/prefork/prefork.go +++ b/prefork/prefork.go @@ -220,7 +220,14 @@ func (p *Prefork) setTCPListenerFiles(addr string) error { func (p *Prefork) doCommand() (*exec.Cmd, error) { // Use custom CommandProducer if provided if p.CommandProducer != nil { - return p.CommandProducer(p.files) + cmd, err := p.CommandProducer(p.files) + if err != nil { + return nil, err + } + if cmd == nil || cmd.Process == nil { + return nil, errors.New("prefork: CommandProducer must return a started command") + } + return cmd, nil } // Default implementation using os.Executable() for reliable path resolution @@ -366,7 +373,11 @@ func (p *Prefork) ListenAndServe(addr string) error { // ListenAndServeTLS serves HTTPS requests from the given TCP addr. // -// certKey and certFile are paths to TLS key and certificate files. +// certKey is the path to the TLS private key file. +// certFile is the path to the TLS certificate file. +// +// Note: parameter order is (addr, certKey, certFile) — key before cert. +// Internally forwards to ServeTLSFunc as (certFile, certKey). func (p *Prefork) ListenAndServeTLS(addr, certKey, certFile string) error { if IsChild() { ln, err := p.listenAsChild(addr) diff --git a/prefork/prefork_test.go b/prefork/prefork_test.go index 39d1449dff..9240f49f7f 100644 --- a/prefork/prefork_test.go +++ b/prefork/prefork_test.go @@ -1,6 +1,7 @@ package prefork import ( + "errors" "fmt" "math/rand" "net" @@ -259,18 +260,15 @@ func Test_Prefork_OnMasterDeath(t *testing.T) { var called bool p := &Prefork{ - Reuseport: true, OnMasterDeath: func() { called = true }, } - // Verify OnMasterDeath is set if p.OnMasterDeath == nil { t.Error("OnMasterDeath should not be nil") } - // Verify it can be called p.OnMasterDeath() if !called { t.Error("OnMasterDeath was not called") @@ -384,15 +382,14 @@ func Test_ErrOnlyReuseportOnWindows(t *testing.T) { } } -func Test_Listen_WithOnMasterDeath(t *testing.T) { +func Test_Listen_ChildCreatesListener(t *testing.T) { // This test can't run parallel as it modifies env. setUp() defer tearDown() p := &Prefork{ - Reuseport: true, - OnMasterDeath: func() { os.Exit(1) }, + Reuseport: true, } addr := getAddr() @@ -402,7 +399,6 @@ func Test_Listen_WithOnMasterDeath(t *testing.T) { } defer ln.Close() - // Verify listener was created if ln == nil { t.Error("Listener should not be nil") } @@ -411,7 +407,7 @@ func Test_Listen_WithOnMasterDeath(t *testing.T) { func Test_OnChildSpawn_Error(t *testing.T) { t.Parallel() - errExpected := fmt.Errorf("spawn callback error") + errExpected := errors.New("spawn callback error") p := &Prefork{ OnChildSpawn: func(pid int) error { return errExpected @@ -428,7 +424,7 @@ func Test_OnChildSpawn_Error(t *testing.T) { func Test_OnMasterReady_Error(t *testing.T) { t.Parallel() - errExpected := fmt.Errorf("master ready callback error") + errExpected := errors.New("master ready callback error") p := &Prefork{ OnMasterReady: func(childPIDs []int) error { return errExpected @@ -474,26 +470,24 @@ func Test_CommandProducer(t *testing.T) { p := &Prefork{ CommandProducer: func(files []*os.File) (*exec.Cmd, error) { producerCalled = true - // Return a simple command that exits quickly - cmd := exec.Command("go", "version") + // Re-exec the test binary with a no-op flag for hermetic testing + cmd := exec.Command(os.Args[0], "-test.run=^$") cmd.ExtraFiles = files + cmd.Env = append(os.Environ(), preforkChildEnvVariable+"=1") err := cmd.Start() return cmd, err }, } - // Verify CommandProducer is set if p.CommandProducer == nil { t.Error("CommandProducer should not be nil") } - // Call doCommand and verify our producer was used cmd, err := p.doCommand() if err != nil { t.Fatalf("doCommand failed: %v", err) } - // Wait for the command to finish _ = cmd.Wait() if !producerCalled {