fix(app): wait for the broker to exit before kill --broker reports success
- commit
- b71082c9b9edcccebe0bcb16466d9cdc12108c6c
- author
- brice · · 2026-09-23 08:45 UTC
- parent
- 0ec9504
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +75 -0
M internal/app/broker_kill_linux.go +28 -0
| @@ -9,6 +9,7 @@ import ( | |||
| 9 | 9 | "net" | |
| 10 | 10 | "os" | |
| 11 | 11 | "syscall" | |
| 12 | + | "time" | |
| 12 | 13 | ||
| 13 | 14 | "github.com/bnema/vev/internal/adapters/brokeripc" | |
| 14 | 15 | ) | |
| @@ -50,6 +51,33 @@ func runKillBroker(ctx context.Context) error { | |||
| 50 | 51 | if err := syscall.Kill(int(peer.Pid), syscall.SIGTERM); err != nil { | |
| 51 | 52 | return fmt.Errorf("vev: stop broker: %w", err) | |
| 52 | 53 | } | |
| 54 | + | // Report success only once the process is gone, so a command that follows | |
| 55 | + | // immediately never reaches the draining broker. | |
| 56 | + | if err := waitProcessExit(ctx, int(peer.Pid), brokerStopTimeout); err != nil { | |
| 57 | + | return fmt.Errorf("vev: stop broker (pid %d): %w", peer.Pid, err) | |
| 58 | + | } | |
| 53 | 59 | fmt.Println("stopped broker") | |
| 54 | 60 | return nil | |
| 55 | 61 | } | |
| 62 | + | ||
| 63 | + | // brokerStopTimeout bounds how long kill --broker waits for an orderly exit. | |
| 64 | + | const brokerStopTimeout = 5 * time.Second | |
| 65 | + | ||
| 66 | + | // waitProcessExit polls until pid no longer exists. The broker is reparented | |
| 67 | + | // at launch, so it is never this process' zombie child. | |
| 68 | + | func waitProcessExit(ctx context.Context, pid int, timeout time.Duration) error { | |
| 69 | + | ctx, cancel := context.WithTimeout(ctx, timeout) | |
| 70 | + | defer cancel() | |
| 71 | + | ticker := time.NewTicker(20 * time.Millisecond) | |
| 72 | + | defer ticker.Stop() | |
| 73 | + | for { | |
| 74 | + | if err := syscall.Kill(pid, 0); errors.Is(err, syscall.ESRCH) { | |
| 75 | + | return nil | |
| 76 | + | } | |
| 77 | + | select { | |
| 78 | + | case <-ctx.Done(): | |
| 79 | + | return fmt.Errorf("still running after %s: %w", timeout, ctx.Err()) | |
| 80 | + | case <-ticker.C: | |
| 81 | + | } | |
| 82 | + | } | |
| 83 | + | } | |
| @@ -9,6 +9,7 @@ import ( | |||||
| 9 | "net" | 9 | "net" | ||
| 10 | "os" | 10 | "os" | ||
| 11 | "syscall" | 11 | "syscall" | ||
| 12 | + | "time" | |||
| 12 | 13 | ||||
| 13 | "github.com/bnema/vev/internal/adapters/brokeripc" | 14 | "github.com/bnema/vev/internal/adapters/brokeripc" | ||
| 14 | ) | 15 | ) | ||
| @@ -50,6 +51,33 @@ func runKillBroker(ctx context.Context) error { | |||||
| 50 | if err := syscall.Kill(int(peer.Pid), syscall.SIGTERM); err != nil { | 51 | if err := syscall.Kill(int(peer.Pid), syscall.SIGTERM); err != nil { | ||
| 51 | return fmt.Errorf("vev: stop broker: %w", err) | 52 | return fmt.Errorf("vev: stop broker: %w", err) | ||
| 52 | } | 53 | } | ||
| 54 | + | // Report success only once the process is gone, so a command that follows | |||
| 55 | + | // immediately never reaches the draining broker. | |||
| 56 | + | if err := waitProcessExit(ctx, int(peer.Pid), brokerStopTimeout); err != nil { | |||
| 57 | + | return fmt.Errorf("vev: stop broker (pid %d): %w", peer.Pid, err) | |||
| 58 | + | } | |||
| 53 | fmt.Println("stopped broker") | 59 | fmt.Println("stopped broker") | ||
| 54 | return nil | 60 | return nil | ||
| 55 | } | 61 | } | ||
| 62 | + | ||||
| 63 | + | // brokerStopTimeout bounds how long kill --broker waits for an orderly exit. | |||
| 64 | + | const brokerStopTimeout = 5 * time.Second | |||
| 65 | + | ||||
| 66 | + | // waitProcessExit polls until pid no longer exists. The broker is reparented | |||
| 67 | + | // at launch, so it is never this process' zombie child. | |||
| 68 | + | func waitProcessExit(ctx context.Context, pid int, timeout time.Duration) error { | |||
| 69 | + | ctx, cancel := context.WithTimeout(ctx, timeout) | |||
| 70 | + | defer cancel() | |||
| 71 | + | ticker := time.NewTicker(20 * time.Millisecond) | |||
| 72 | + | defer ticker.Stop() | |||
| 73 | + | for { | |||
| 74 | + | if err := syscall.Kill(pid, 0); errors.Is(err, syscall.ESRCH) { | |||
| 75 | + | return nil | |||
| 76 | + | } | |||
| 77 | + | select { | |||
| 78 | + | case <-ctx.Done(): | |||
| 79 | + | return fmt.Errorf("still running after %s: %w", timeout, ctx.Err()) | |||
| 80 | + | case <-ticker.C: | |||
| 81 | + | } | |||
| 82 | + | } | |||
| 83 | + | } | |||
A internal/app/broker_kill_linux_test.go +47 -0
| @@ -0,0 +1,47 @@ | |||
| 1 | + | //go:build linux | |
| 2 | + | ||
| 3 | + | package app | |
| 4 | + | ||
| 5 | + | import ( | |
| 6 | + | "context" | |
| 7 | + | "os/exec" | |
| 8 | + | "syscall" | |
| 9 | + | "testing" | |
| 10 | + | "time" | |
| 11 | + | ||
| 12 | + | "github.com/stretchr/testify/require" | |
| 13 | + | ) | |
| 14 | + | ||
| 15 | + | func TestWaitProcessExit(t *testing.T) { | |
| 16 | + | tests := []struct { | |
| 17 | + | name string | |
| 18 | + | signal bool | |
| 19 | + | wantErr bool | |
| 20 | + | }{ | |
| 21 | + | {name: "terminated process is reported gone", signal: true}, | |
| 22 | + | {name: "live process times out", signal: false, wantErr: true}, | |
| 23 | + | } | |
| 24 | + | for _, tt := range tests { | |
| 25 | + | t.Run(tt.name, func(t *testing.T) { | |
| 26 | + | cmd := exec.Command("sleep", "30") | |
| 27 | + | require.NoError(t, cmd.Start()) | |
| 28 | + | // Reap the child so it never lingers as a zombie that kill(pid, 0) | |
| 29 | + | // would still report as present. | |
| 30 | + | reaped := make(chan struct{}) | |
| 31 | + | go func() { _ = cmd.Wait(); close(reaped) }() | |
| 32 | + | t.Cleanup(func() { | |
| 33 | + | _ = cmd.Process.Kill() | |
| 34 | + | <-reaped | |
| 35 | + | }) | |
| 36 | + | if tt.signal { | |
| 37 | + | require.NoError(t, syscall.Kill(cmd.Process.Pid, syscall.SIGTERM)) | |
| 38 | + | } | |
| 39 | + | err := waitProcessExit(context.Background(), cmd.Process.Pid, 300*time.Millisecond) | |
| 40 | + | if tt.wantErr { | |
| 41 | + | require.Error(t, err) | |
| 42 | + | return | |
| 43 | + | } | |
| 44 | + | require.NoError(t, err) | |
| 45 | + | }) | |
| 46 | + | } | |
| 47 | + | } | |
| @@ -0,0 +1,47 @@ | |||||
| 1 | + | //go:build linux | |||
| 2 | + | ||||
| 3 | + | package app | |||
| 4 | + | ||||
| 5 | + | import ( | |||
| 6 | + | "context" | |||
| 7 | + | "os/exec" | |||
| 8 | + | "syscall" | |||
| 9 | + | "testing" | |||
| 10 | + | "time" | |||
| 11 | + | ||||
| 12 | + | "github.com/stretchr/testify/require" | |||
| 13 | + | ) | |||
| 14 | + | ||||
| 15 | + | func TestWaitProcessExit(t *testing.T) { | |||
| 16 | + | tests := []struct { | |||
| 17 | + | name string | |||
| 18 | + | signal bool | |||
| 19 | + | wantErr bool | |||
| 20 | + | }{ | |||
| 21 | + | {name: "terminated process is reported gone", signal: true}, | |||
| 22 | + | {name: "live process times out", signal: false, wantErr: true}, | |||
| 23 | + | } | |||
| 24 | + | for _, tt := range tests { | |||
| 25 | + | t.Run(tt.name, func(t *testing.T) { | |||
| 26 | + | cmd := exec.Command("sleep", "30") | |||
| 27 | + | require.NoError(t, cmd.Start()) | |||
| 28 | + | // Reap the child so it never lingers as a zombie that kill(pid, 0) | |||
| 29 | + | // would still report as present. | |||
| 30 | + | reaped := make(chan struct{}) | |||
| 31 | + | go func() { _ = cmd.Wait(); close(reaped) }() | |||
| 32 | + | t.Cleanup(func() { | |||
| 33 | + | _ = cmd.Process.Kill() | |||
| 34 | + | <-reaped | |||
| 35 | + | }) | |||
| 36 | + | if tt.signal { | |||
| 37 | + | require.NoError(t, syscall.Kill(cmd.Process.Pid, syscall.SIGTERM)) | |||
| 38 | + | } | |||
| 39 | + | err := waitProcessExit(context.Background(), cmd.Process.Pid, 300*time.Millisecond) | |||
| 40 | + | if tt.wantErr { | |||
| 41 | + | require.Error(t, err) | |||
| 42 | + | return | |||
| 43 | + | } | |||
| 44 | + | require.NoError(t, err) | |||
| 45 | + | }) | |||
| 46 | + | } | |||
| 47 | + | } | |||