fix(hints): kill the detector's process group on timeout
- commit
- e8e812d40143788450ff10d498dd868c926347c6
- author
- brice · · 2026-10-21 13:38 UTC
- committer
- brice · · 2026-10-22 20:27 UTC
- parent
- 88edc52
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +239 -22
M internal/adapters/hints/detector.go +67 -22
| @@ -9,9 +9,11 @@ import ( | |||
| 9 | 9 | "fmt" | |
| 10 | 10 | "io" | |
| 11 | 11 | "math" | |
| 12 | + | "os" | |
| 12 | 13 | "os/exec" | |
| 13 | 14 | "strconv" | |
| 14 | 15 | "sync" | |
| 16 | + | "syscall" | |
| 15 | 17 | "time" | |
| 16 | 18 | ||
| 17 | 19 | "github.com/bnema/neferwl/internal/adapters/logging" | |
| @@ -203,48 +205,88 @@ func newProcDetector(command, model string, threshold float64) *procDetector { | |||
| 203 | 205 | return &procDetector{command: command, model: model, conf: threshold, helloTimeout: helloTimeout, reqTimeout: requestTimeout} | |
| 204 | 206 | } | |
| 205 | 207 | ||
| 206 | - | // process is one running child. | |
| 208 | + | // waitDelay bounds cmd.Wait once the child is dead: a descendant that kept | |
| 209 | + | // stderr open must not block the reaping. | |
| 210 | + | const ( | |
| 211 | + | waitDelay = time.Second | |
| 212 | + | closeGrace = time.Second | |
| 213 | + | ) | |
| 214 | + | ||
| 215 | + | // process is one running child, alone in its process group. It keeps both | |
| 216 | + | // pipe ends so that closing them unblocks a pending read or write. | |
| 207 | 217 | type process struct { | |
| 208 | - | cmd *exec.Cmd | |
| 209 | - | in io.WriteCloser | |
| 210 | - | out *bufio.Reader | |
| 211 | - | once sync.Once | |
| 218 | + | cmd *exec.Cmd | |
| 219 | + | in *os.File // write end of the child's stdin | |
| 220 | + | out *os.File // read end of the child's stdout | |
| 221 | + | rd *bufio.Reader | |
| 222 | + | ||
| 223 | + | mu sync.Mutex | |
| 224 | + | reaped bool | |
| 225 | + | once sync.Once | |
| 226 | + | } | |
| 227 | + | ||
| 228 | + | // interrupt kills the whole process group and closes our pipe ends, without | |
| 229 | + | // waiting: it unblocks a pending read or write from a timer or a context | |
| 230 | + | // callback. kill reaps the child afterwards. | |
| 231 | + | func (p *process) interrupt() { | |
| 232 | + | p.mu.Lock() | |
| 233 | + | if !p.reaped { | |
| 234 | + | _ = syscall.Kill(-p.cmd.Process.Pid, syscall.SIGKILL) | |
| 235 | + | } | |
| 236 | + | p.mu.Unlock() | |
| 237 | + | _ = p.out.Close() | |
| 238 | + | _ = p.in.Close() | |
| 212 | 239 | } | |
| 213 | 240 | ||
| 214 | - | // kill stops the child and reaps it; safe to call more than once. | |
| 241 | + | // kill stops the child and its descendants and reaps it; safe to call more | |
| 242 | + | // than once. Wait is bounded by cmd.WaitDelay. | |
| 215 | 243 | func (p *process) kill() { | |
| 244 | + | p.interrupt() | |
| 245 | + | p.wait() | |
| 246 | + | } | |
| 247 | + | ||
| 248 | + | // wait reaps the child (once); bounded by cmd.WaitDelay after its death. | |
| 249 | + | func (p *process) wait() { | |
| 216 | 250 | p.once.Do(func() { | |
| 217 | - | _ = p.cmd.Process.Kill() | |
| 218 | - | _ = p.in.Close() | |
| 219 | 251 | _ = p.cmd.Wait() | |
| 252 | + | p.mu.Lock() | |
| 253 | + | p.reaped = true | |
| 254 | + | p.mu.Unlock() | |
| 220 | 255 | }) | |
| 221 | 256 | } | |
| 222 | 257 | ||
| 223 | - | // interrupt kills the child without waiting: it unblocks a pending read or | |
| 224 | - | // write from a timer or a context callback. kill reaps it afterwards. | |
| 225 | - | func (p *process) interrupt() { _ = p.cmd.Process.Kill() } | |
| 226 | - | ||
| 227 | 258 | func (d *procDetector) start(ctx context.Context) error { | |
| 228 | 259 | log := logging.For(ctx, "hints") | |
| 229 | 260 | cmd := exec.Command(d.command, "--model", d.model, "--imgsz", strconv.Itoa(imgsz), | |
| 230 | 261 | "--conf", strconv.FormatFloat(d.conf, 'f', -1, 64), "--gpu", "auto") | |
| 231 | 262 | cmd.Stderr = &lineLog{log: func(line string) { log.Debug().Str("detector", d.command).Msg(line) }} | |
| 232 | - | in, err := cmd.StdinPipe() | |
| 263 | + | cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} | |
| 264 | + | cmd.WaitDelay = waitDelay | |
| 265 | + | inR, inW, err := os.Pipe() | |
| 233 | 266 | if err != nil { | |
| 234 | 267 | return err | |
| 235 | 268 | } | |
| 236 | - | out, err := cmd.StdoutPipe() | |
| 269 | + | outR, outW, err := os.Pipe() | |
| 237 | 270 | if err != nil { | |
| 271 | + | _ = inR.Close() | |
| 272 | + | _ = inW.Close() | |
| 238 | 273 | return err | |
| 239 | 274 | } | |
| 240 | - | if err := cmd.Start(); err != nil { | |
| 275 | + | cmd.Stdin, cmd.Stdout = inR, outW | |
| 276 | + | err = cmd.Start() | |
| 277 | + | // The child has its ends; ours are not needed any more. | |
| 278 | + | _ = inR.Close() | |
| 279 | + | _ = outW.Close() | |
| 280 | + | if err != nil { | |
| 281 | + | _ = inW.Close() | |
| 282 | + | _ = outR.Close() | |
| 241 | 283 | return fmt.Errorf("hints: start detector: %w", err) | |
| 242 | 284 | } | |
| 243 | - | p := &process{cmd: cmd, in: in, out: bufio.NewReaderSize(out, 1<<16)} | |
| 285 | + | p := &process{cmd: cmd, in: inW, out: outR, rd: bufio.NewReaderSize(outR, 1<<16)} | |
| 244 | 286 | t0 := time.Now() | |
| 245 | 287 | timer := time.AfterFunc(d.helloTimeout, p.interrupt) | |
| 246 | 288 | stop := context.AfterFunc(ctx, p.interrupt) | |
| 247 | - | device, err := readHello(p.out) | |
| 289 | + | device, err := readHello(p.rd) | |
| 248 | 290 | stop() | |
| 249 | 291 | timedOut := !timer.Stop() | |
| 250 | 292 | if err != nil { | |
| @@ -296,7 +338,7 @@ func (d *procDetector) roundTrip(p *process, f detFrame) ([]detection, error) { | |||
| 296 | 338 | if err := bw.Flush(); err != nil { | |
| 297 | 339 | return nil, err | |
| 298 | 340 | } | |
| 299 | - | return readResponse(p.out, d.seq) | |
| 341 | + | return readResponse(p.rd, d.seq) | |
| 300 | 342 | } | |
| 301 | 343 | ||
| 302 | 344 | // drop kills the current child: the next Detect starts a new one. | |
| @@ -317,12 +359,15 @@ func (d *procDetector) Close() error { | |||
| 317 | 359 | } | |
| 318 | 360 | _ = p.in.Close() | |
| 319 | 361 | done := make(chan struct{}) | |
| 320 | - | go func() { _ = p.cmd.Wait(); close(done) }() | |
| 362 | + | go func() { p.wait(); close(done) }() | |
| 321 | 363 | select { | |
| 322 | 364 | case <-done: | |
| 323 | - | case <-time.After(time.Second): | |
| 324 | - | _ = p.cmd.Process.Kill() | |
| 325 | - | <-done | |
| 365 | + | case <-time.After(closeGrace): | |
| 366 | + | p.interrupt() | |
| 367 | + | select { | |
| 368 | + | case <-done: | |
| 369 | + | case <-time.After(2 * waitDelay): | |
| 370 | + | } | |
| 326 | 371 | } | |
| 327 | 372 | return nil | |
| 328 | 373 | } | |
| @@ -9,9 +9,11 @@ import ( | |||||
| 9 | "fmt" | 9 | "fmt" | ||
| 10 | "io" | 10 | "io" | ||
| 11 | "math" | 11 | "math" | ||
| 12 | + | "os" | |||
| 12 | "os/exec" | 13 | "os/exec" | ||
| 13 | "strconv" | 14 | "strconv" | ||
| 14 | "sync" | 15 | "sync" | ||
| 16 | + | "syscall" | |||
| 15 | "time" | 17 | "time" | ||
| 16 | 18 | ||||
| 17 | "github.com/bnema/neferwl/internal/adapters/logging" | 19 | "github.com/bnema/neferwl/internal/adapters/logging" | ||
| @@ -203,48 +205,88 @@ func newProcDetector(command, model string, threshold float64) *procDetector { | |||||
| 203 | return &procDetector{command: command, model: model, conf: threshold, helloTimeout: helloTimeout, reqTimeout: requestTimeout} | 205 | return &procDetector{command: command, model: model, conf: threshold, helloTimeout: helloTimeout, reqTimeout: requestTimeout} | ||
| 204 | } | 206 | } | ||
| 205 | 207 | ||||
| 206 | - | // process is one running child. | 208 | + | // waitDelay bounds cmd.Wait once the child is dead: a descendant that kept |
| 209 | + | // stderr open must not block the reaping. | |||
| 210 | + | const ( | |||
| 211 | + | waitDelay = time.Second | |||
| 212 | + | closeGrace = time.Second | |||
| 213 | + | ) | |||
| 214 | + | ||||
| 215 | + | // process is one running child, alone in its process group. It keeps both | |||
| 216 | + | // pipe ends so that closing them unblocks a pending read or write. | |||
| 207 | type process struct { | 217 | type process struct { | ||
| 208 | - | cmd *exec.Cmd | 218 | + | cmd *exec.Cmd |
| 209 | - | in io.WriteCloser | 219 | + | in *os.File // write end of the child's stdin |
| 210 | - | out *bufio.Reader | 220 | + | out *os.File // read end of the child's stdout |
| 211 | - | once sync.Once | 221 | + | rd *bufio.Reader |
| 222 | + | ||||
| 223 | + | mu sync.Mutex | |||
| 224 | + | reaped bool | |||
| 225 | + | once sync.Once | |||
| 226 | + | } | |||
| 227 | + | ||||
| 228 | + | // interrupt kills the whole process group and closes our pipe ends, without | |||
| 229 | + | // waiting: it unblocks a pending read or write from a timer or a context | |||
| 230 | + | // callback. kill reaps the child afterwards. | |||
| 231 | + | func (p *process) interrupt() { | |||
| 232 | + | p.mu.Lock() | |||
| 233 | + | if !p.reaped { | |||
| 234 | + | _ = syscall.Kill(-p.cmd.Process.Pid, syscall.SIGKILL) | |||
| 235 | + | } | |||
| 236 | + | p.mu.Unlock() | |||
| 237 | + | _ = p.out.Close() | |||
| 238 | + | _ = p.in.Close() | |||
| 212 | } | 239 | } | ||
| 213 | 240 | ||||
| 214 | - | // kill stops the child and reaps it; safe to call more than once. | 241 | + | // kill stops the child and its descendants and reaps it; safe to call more |
| 242 | + | // than once. Wait is bounded by cmd.WaitDelay. | |||
| 215 | func (p *process) kill() { | 243 | func (p *process) kill() { | ||
| 244 | + | p.interrupt() | |||
| 245 | + | p.wait() | |||
| 246 | + | } | |||
| 247 | + | ||||
| 248 | + | // wait reaps the child (once); bounded by cmd.WaitDelay after its death. | |||
| 249 | + | func (p *process) wait() { | |||
| 216 | p.once.Do(func() { | 250 | p.once.Do(func() { | ||
| 217 | - | _ = p.cmd.Process.Kill() | |||
| 218 | - | _ = p.in.Close() | |||
| 219 | _ = p.cmd.Wait() | 251 | _ = p.cmd.Wait() | ||
| 252 | + | p.mu.Lock() | |||
| 253 | + | p.reaped = true | |||
| 254 | + | p.mu.Unlock() | |||
| 220 | }) | 255 | }) | ||
| 221 | } | 256 | } | ||
| 222 | 257 | ||||
| 223 | - | // interrupt kills the child without waiting: it unblocks a pending read or | |||
| 224 | - | // write from a timer or a context callback. kill reaps it afterwards. | |||
| 225 | - | func (p *process) interrupt() { _ = p.cmd.Process.Kill() } | |||
| 226 | - | ||||
| 227 | func (d *procDetector) start(ctx context.Context) error { | 258 | func (d *procDetector) start(ctx context.Context) error { | ||
| 228 | log := logging.For(ctx, "hints") | 259 | log := logging.For(ctx, "hints") | ||
| 229 | cmd := exec.Command(d.command, "--model", d.model, "--imgsz", strconv.Itoa(imgsz), | 260 | cmd := exec.Command(d.command, "--model", d.model, "--imgsz", strconv.Itoa(imgsz), | ||
| 230 | "--conf", strconv.FormatFloat(d.conf, 'f', -1, 64), "--gpu", "auto") | 261 | "--conf", strconv.FormatFloat(d.conf, 'f', -1, 64), "--gpu", "auto") | ||
| 231 | cmd.Stderr = &lineLog{log: func(line string) { log.Debug().Str("detector", d.command).Msg(line) }} | 262 | cmd.Stderr = &lineLog{log: func(line string) { log.Debug().Str("detector", d.command).Msg(line) }} | ||
| 232 | - | in, err := cmd.StdinPipe() | 263 | + | cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} |
| 264 | + | cmd.WaitDelay = waitDelay | |||
| 265 | + | inR, inW, err := os.Pipe() | |||
| 233 | if err != nil { | 266 | if err != nil { | ||
| 234 | return err | 267 | return err | ||
| 235 | } | 268 | } | ||
| 236 | - | out, err := cmd.StdoutPipe() | 269 | + | outR, outW, err := os.Pipe() |
| 237 | if err != nil { | 270 | if err != nil { | ||
| 271 | + | _ = inR.Close() | |||
| 272 | + | _ = inW.Close() | |||
| 238 | return err | 273 | return err | ||
| 239 | } | 274 | } | ||
| 240 | - | if err := cmd.Start(); err != nil { | 275 | + | cmd.Stdin, cmd.Stdout = inR, outW |
| 276 | + | err = cmd.Start() | |||
| 277 | + | // The child has its ends; ours are not needed any more. | |||
| 278 | + | _ = inR.Close() | |||
| 279 | + | _ = outW.Close() | |||
| 280 | + | if err != nil { | |||
| 281 | + | _ = inW.Close() | |||
| 282 | + | _ = outR.Close() | |||
| 241 | return fmt.Errorf("hints: start detector: %w", err) | 283 | return fmt.Errorf("hints: start detector: %w", err) | ||
| 242 | } | 284 | } | ||
| 243 | - | p := &process{cmd: cmd, in: in, out: bufio.NewReaderSize(out, 1<<16)} | 285 | + | p := &process{cmd: cmd, in: inW, out: outR, rd: bufio.NewReaderSize(outR, 1<<16)} |
| 244 | t0 := time.Now() | 286 | t0 := time.Now() | ||
| 245 | timer := time.AfterFunc(d.helloTimeout, p.interrupt) | 287 | timer := time.AfterFunc(d.helloTimeout, p.interrupt) | ||
| 246 | stop := context.AfterFunc(ctx, p.interrupt) | 288 | stop := context.AfterFunc(ctx, p.interrupt) | ||
| 247 | - | device, err := readHello(p.out) | 289 | + | device, err := readHello(p.rd) |
| 248 | stop() | 290 | stop() | ||
| 249 | timedOut := !timer.Stop() | 291 | timedOut := !timer.Stop() | ||
| 250 | if err != nil { | 292 | if err != nil { | ||
| @@ -296,7 +338,7 @@ func (d *procDetector) roundTrip(p *process, f detFrame) ([]detection, error) { | |||||
| 296 | if err := bw.Flush(); err != nil { | 338 | if err := bw.Flush(); err != nil { | ||
| 297 | return nil, err | 339 | return nil, err | ||
| 298 | } | 340 | } | ||
| 299 | - | return readResponse(p.out, d.seq) | 341 | + | return readResponse(p.rd, d.seq) |
| 300 | } | 342 | } | ||
| 301 | 343 | ||||
| 302 | // drop kills the current child: the next Detect starts a new one. | 344 | // drop kills the current child: the next Detect starts a new one. | ||
| @@ -317,12 +359,15 @@ func (d *procDetector) Close() error { | |||||
| 317 | } | 359 | } | ||
| 318 | _ = p.in.Close() | 360 | _ = p.in.Close() | ||
| 319 | done := make(chan struct{}) | 361 | done := make(chan struct{}) | ||
| 320 | - | go func() { _ = p.cmd.Wait(); close(done) }() | 362 | + | go func() { p.wait(); close(done) }() |
| 321 | select { | 363 | select { | ||
| 322 | case <-done: | 364 | case <-done: | ||
| 323 | - | case <-time.After(time.Second): | 365 | + | case <-time.After(closeGrace): |
| 324 | - | _ = p.cmd.Process.Kill() | 366 | + | p.interrupt() |
| 325 | - | <-done | 367 | + | select { |
| 368 | + | case <-done: | |||
| 369 | + | case <-time.After(2 * waitDelay): | |||
| 370 | + | } | |||
| 326 | } | 371 | } | ||
| 327 | return nil | 372 | return nil | ||
| 328 | } | 373 | } | ||
A internal/adapters/hints/detector_proc_test.go +172 -0
| @@ -0,0 +1,172 @@ | |||
| 1 | + | package hints | |
| 2 | + | ||
| 3 | + | import ( | |
| 4 | + | "context" | |
| 5 | + | "io" | |
| 6 | + | "os" | |
| 7 | + | "os/exec" | |
| 8 | + | "strconv" | |
| 9 | + | "strings" | |
| 10 | + | "testing" | |
| 11 | + | "time" | |
| 12 | + | ||
| 13 | + | "github.com/stretchr/testify/require" | |
| 14 | + | ) | |
| 15 | + | ||
| 16 | + | // The detector child of these tests is the test binary itself, re-executed: | |
| 17 | + | // a real process, started with the detector's own arguments. TestMain turns | |
| 18 | + | // it into helperChild when helperEnv is set. | |
| 19 | + | const ( | |
| 20 | + | helperEnv = "NEFERWL_HINTS_HELPER" | |
| 21 | + | helperPIDFile = "NEFERWL_HINTS_HELPER_PIDFILE" | |
| 22 | + | helperChildPID = "NEFERWL_HINTS_HELPER_CHILDPID" | |
| 23 | + | ) | |
| 24 | + | ||
| 25 | + | func TestMain(m *testing.M) { | |
| 26 | + | if mode := os.Getenv(helperEnv); mode != "" { | |
| 27 | + | helperChild(mode) | |
| 28 | + | os.Exit(0) | |
| 29 | + | } | |
| 30 | + | os.Exit(m.Run()) | |
| 31 | + | } | |
| 32 | + | ||
| 33 | + | func helperHello() []byte { | |
| 34 | + | return cat(magicHello[:], u32(protocolVersion), u32(0), u32(4), []byte("test"), u32(imgsz)) | |
| 35 | + | } | |
| 36 | + | ||
| 37 | + | // helperChild plays the detector for mode: | |
| 38 | + | // | |
| 39 | + | // nohello never says hello | |
| 40 | + | // silent says hello, then never answers | |
| 41 | + | // grandchild like nohello, after spawning a grandchild that shares the | |
| 42 | + | // child's stdout, stderr and process group | |
| 43 | + | // ok says hello and answers every request with no box | |
| 44 | + | func helperChild(mode string) { | |
| 45 | + | if f := os.Getenv(helperChildPID); f != "" { | |
| 46 | + | _ = os.WriteFile(f, []byte(strconv.Itoa(os.Getpid())), 0o600) | |
| 47 | + | } | |
| 48 | + | if mode == "grandchild" { | |
| 49 | + | gc := exec.Command("sleep", "60") | |
| 50 | + | gc.Stdout, gc.Stderr = os.Stdout, os.Stderr | |
| 51 | + | if err := gc.Start(); err != nil { | |
| 52 | + | os.Exit(2) | |
| 53 | + | } | |
| 54 | + | _ = os.WriteFile(os.Getenv(helperPIDFile), []byte(strconv.Itoa(gc.Process.Pid)), 0o600) | |
| 55 | + | mode = "nohello" | |
| 56 | + | } | |
| 57 | + | if mode != "nohello" { | |
| 58 | + | _, _ = os.Stdout.Write(helperHello()) | |
| 59 | + | } | |
| 60 | + | if mode != "ok" { | |
| 61 | + | time.Sleep(time.Minute) | |
| 62 | + | return | |
| 63 | + | } | |
| 64 | + | for { | |
| 65 | + | var h [40]byte | |
| 66 | + | if _, err := io.ReadFull(os.Stdin, h[:]); err != nil { | |
| 67 | + | return | |
| 68 | + | } | |
| 69 | + | _, err := io.CopyN(io.Discard, os.Stdin, int64(le.Uint32(h[20:])*le.Uint32(h[24:]))) | |
| 70 | + | if err != nil { | |
| 71 | + | return | |
| 72 | + | } | |
| 73 | + | _, _ = os.Stdout.Write(cat(magicResponse[:], u32(protocolVersion), h[8:16], u32(0), u32(0), u32(0))) | |
| 74 | + | } | |
| 75 | + | } | |
| 76 | + | ||
| 77 | + | func helperDetector(t *testing.T, mode string) *procDetector { | |
| 78 | + | t.Helper() | |
| 79 | + | t.Setenv(helperEnv, mode) | |
| 80 | + | d := newProcDetector(os.Args[0], t.TempDir(), 0.05) | |
| 81 | + | d.helloTimeout, d.reqTimeout = 300*time.Millisecond, 300*time.Millisecond | |
| 82 | + | t.Cleanup(func() { _ = d.Close() }) | |
| 83 | + | return d | |
| 84 | + | } | |
| 85 | + | ||
| 86 | + | func tinyFrame() detFrame { | |
| 87 | + | return detFrame{width: 1, height: 1, stride: 4, m: &mapping{b: make([]byte, 4), live: new(int)}} | |
| 88 | + | } | |
| 89 | + | ||
| 90 | + | // processGone reports whether pid is dead (or a zombie waiting for its | |
| 91 | + | // reaper). | |
| 92 | + | func processGone(pid int) bool { | |
| 93 | + | b, err := os.ReadFile("/proc/" + strconv.Itoa(pid) + "/stat") | |
| 94 | + | if err != nil { | |
| 95 | + | return true | |
| 96 | + | } | |
| 97 | + | s := string(b) | |
| 98 | + | return strings.HasPrefix(s[strings.LastIndexByte(s, ')')+2:], "Z") | |
| 99 | + | } | |
| 100 | + | ||
| 101 | + | func readPID(t *testing.T, file string) int { | |
| 102 | + | t.Helper() | |
| 103 | + | var pid int | |
| 104 | + | require.Eventually(t, func() bool { | |
| 105 | + | b, err := os.ReadFile(file) | |
| 106 | + | if err != nil { | |
| 107 | + | return false | |
| 108 | + | } | |
| 109 | + | pid, err = strconv.Atoi(string(b)) | |
| 110 | + | return err == nil | |
| 111 | + | }, time.Second, 10*time.Millisecond) | |
| 112 | + | return pid | |
| 113 | + | } | |
| 114 | + | ||
| 115 | + | func TestProcDetectorHungChildIsKilledAndRestarted(t *testing.T) { | |
| 116 | + | for _, mode := range []string{"nohello", "silent"} { | |
| 117 | + | t.Run(mode, func(t *testing.T) { | |
| 118 | + | pidFile := t.TempDir() + "/child" | |
| 119 | + | t.Setenv(helperChildPID, pidFile) | |
| 120 | + | d := helperDetector(t, mode) | |
| 121 | + | ctx := context.Background() | |
| 122 | + | start := time.Now() | |
| 123 | + | _, err := d.Detect(ctx, tinyFrame()) | |
| 124 | + | require.ErrorContains(t, err, "timed out") | |
| 125 | + | if mode == "nohello" { | |
| 126 | + | require.ErrorContains(t, err, "hello") | |
| 127 | + | } | |
| 128 | + | require.Less(t, time.Since(start), 2*time.Second) | |
| 129 | + | require.Nil(t, d.proc) | |
| 130 | + | pid := readPID(t, pidFile) | |
| 131 | + | require.True(t, processGone(pid), "child %d survives", pid) | |
| 132 | + | ||
| 133 | + | // The next Detect starts a new, healthy child. | |
| 134 | + | t.Setenv(helperEnv, "ok") | |
| 135 | + | t.Setenv(helperChildPID, "") | |
| 136 | + | _, err = d.Detect(ctx, tinyFrame()) | |
| 137 | + | require.NoError(t, err) | |
| 138 | + | require.NotNil(t, d.proc) | |
| 139 | + | start = time.Now() | |
| 140 | + | require.NoError(t, d.Close()) | |
| 141 | + | require.Less(t, time.Since(start), 3*time.Second) | |
| 142 | + | }) | |
| 143 | + | } | |
| 144 | + | } | |
| 145 | + | ||
| 146 | + | // A grandchild holding the child's stdout and stderr open must neither | |
| 147 | + | // survive the kill nor keep Wait blocked. | |
| 148 | + | func TestProcDetectorKillsGrandchild(t *testing.T) { | |
| 149 | + | pidFile := t.TempDir() + "/pid" | |
| 150 | + | t.Setenv(helperPIDFile, pidFile) | |
| 151 | + | d := helperDetector(t, "grandchild") | |
| 152 | + | start := time.Now() | |
| 153 | + | _, err := d.Detect(context.Background(), tinyFrame()) | |
| 154 | + | require.ErrorContains(t, err, "hello timed out") | |
| 155 | + | require.Less(t, time.Since(start), 2500*time.Millisecond) | |
| 156 | + | pid := readPID(t, pidFile) | |
| 157 | + | require.Eventually(t, func() bool { return processGone(pid) }, 2*time.Second, 20*time.Millisecond, "grandchild %d survives", pid) | |
| 158 | + | require.Nil(t, d.proc) | |
| 159 | + | } | |
| 160 | + | ||
| 161 | + | // Cancelling the context interrupts a pending read. | |
| 162 | + | func TestProcDetectorContextCancel(t *testing.T) { | |
| 163 | + | d := helperDetector(t, "silent") | |
| 164 | + | d.reqTimeout = time.Minute | |
| 165 | + | ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) | |
| 166 | + | defer cancel() | |
| 167 | + | start := time.Now() | |
| 168 | + | _, err := d.Detect(ctx, tinyFrame()) | |
| 169 | + | require.ErrorIs(t, err, context.DeadlineExceeded) | |
| 170 | + | require.Less(t, time.Since(start), 2*time.Second) | |
| 171 | + | require.Nil(t, d.proc) | |
| 172 | + | } | |
| @@ -0,0 +1,172 @@ | |||||
| 1 | + | package hints | |||
| 2 | + | ||||
| 3 | + | import ( | |||
| 4 | + | "context" | |||
| 5 | + | "io" | |||
| 6 | + | "os" | |||
| 7 | + | "os/exec" | |||
| 8 | + | "strconv" | |||
| 9 | + | "strings" | |||
| 10 | + | "testing" | |||
| 11 | + | "time" | |||
| 12 | + | ||||
| 13 | + | "github.com/stretchr/testify/require" | |||
| 14 | + | ) | |||
| 15 | + | ||||
| 16 | + | // The detector child of these tests is the test binary itself, re-executed: | |||
| 17 | + | // a real process, started with the detector's own arguments. TestMain turns | |||
| 18 | + | // it into helperChild when helperEnv is set. | |||
| 19 | + | const ( | |||
| 20 | + | helperEnv = "NEFERWL_HINTS_HELPER" | |||
| 21 | + | helperPIDFile = "NEFERWL_HINTS_HELPER_PIDFILE" | |||
| 22 | + | helperChildPID = "NEFERWL_HINTS_HELPER_CHILDPID" | |||
| 23 | + | ) | |||
| 24 | + | ||||
| 25 | + | func TestMain(m *testing.M) { | |||
| 26 | + | if mode := os.Getenv(helperEnv); mode != "" { | |||
| 27 | + | helperChild(mode) | |||
| 28 | + | os.Exit(0) | |||
| 29 | + | } | |||
| 30 | + | os.Exit(m.Run()) | |||
| 31 | + | } | |||
| 32 | + | ||||
| 33 | + | func helperHello() []byte { | |||
| 34 | + | return cat(magicHello[:], u32(protocolVersion), u32(0), u32(4), []byte("test"), u32(imgsz)) | |||
| 35 | + | } | |||
| 36 | + | ||||
| 37 | + | // helperChild plays the detector for mode: | |||
| 38 | + | // | |||
| 39 | + | // nohello never says hello | |||
| 40 | + | // silent says hello, then never answers | |||
| 41 | + | // grandchild like nohello, after spawning a grandchild that shares the | |||
| 42 | + | // child's stdout, stderr and process group | |||
| 43 | + | // ok says hello and answers every request with no box | |||
| 44 | + | func helperChild(mode string) { | |||
| 45 | + | if f := os.Getenv(helperChildPID); f != "" { | |||
| 46 | + | _ = os.WriteFile(f, []byte(strconv.Itoa(os.Getpid())), 0o600) | |||
| 47 | + | } | |||
| 48 | + | if mode == "grandchild" { | |||
| 49 | + | gc := exec.Command("sleep", "60") | |||
| 50 | + | gc.Stdout, gc.Stderr = os.Stdout, os.Stderr | |||
| 51 | + | if err := gc.Start(); err != nil { | |||
| 52 | + | os.Exit(2) | |||
| 53 | + | } | |||
| 54 | + | _ = os.WriteFile(os.Getenv(helperPIDFile), []byte(strconv.Itoa(gc.Process.Pid)), 0o600) | |||
| 55 | + | mode = "nohello" | |||
| 56 | + | } | |||
| 57 | + | if mode != "nohello" { | |||
| 58 | + | _, _ = os.Stdout.Write(helperHello()) | |||
| 59 | + | } | |||
| 60 | + | if mode != "ok" { | |||
| 61 | + | time.Sleep(time.Minute) | |||
| 62 | + | return | |||
| 63 | + | } | |||
| 64 | + | for { | |||
| 65 | + | var h [40]byte | |||
| 66 | + | if _, err := io.ReadFull(os.Stdin, h[:]); err != nil { | |||
| 67 | + | return | |||
| 68 | + | } | |||
| 69 | + | _, err := io.CopyN(io.Discard, os.Stdin, int64(le.Uint32(h[20:])*le.Uint32(h[24:]))) | |||
| 70 | + | if err != nil { | |||
| 71 | + | return | |||
| 72 | + | } | |||
| 73 | + | _, _ = os.Stdout.Write(cat(magicResponse[:], u32(protocolVersion), h[8:16], u32(0), u32(0), u32(0))) | |||
| 74 | + | } | |||
| 75 | + | } | |||
| 76 | + | ||||
| 77 | + | func helperDetector(t *testing.T, mode string) *procDetector { | |||
| 78 | + | t.Helper() | |||
| 79 | + | t.Setenv(helperEnv, mode) | |||
| 80 | + | d := newProcDetector(os.Args[0], t.TempDir(), 0.05) | |||
| 81 | + | d.helloTimeout, d.reqTimeout = 300*time.Millisecond, 300*time.Millisecond | |||
| 82 | + | t.Cleanup(func() { _ = d.Close() }) | |||
| 83 | + | return d | |||
| 84 | + | } | |||
| 85 | + | ||||
| 86 | + | func tinyFrame() detFrame { | |||
| 87 | + | return detFrame{width: 1, height: 1, stride: 4, m: &mapping{b: make([]byte, 4), live: new(int)}} | |||
| 88 | + | } | |||
| 89 | + | ||||
| 90 | + | // processGone reports whether pid is dead (or a zombie waiting for its | |||
| 91 | + | // reaper). | |||
| 92 | + | func processGone(pid int) bool { | |||
| 93 | + | b, err := os.ReadFile("/proc/" + strconv.Itoa(pid) + "/stat") | |||
| 94 | + | if err != nil { | |||
| 95 | + | return true | |||
| 96 | + | } | |||
| 97 | + | s := string(b) | |||
| 98 | + | return strings.HasPrefix(s[strings.LastIndexByte(s, ')')+2:], "Z") | |||
| 99 | + | } | |||
| 100 | + | ||||
| 101 | + | func readPID(t *testing.T, file string) int { | |||
| 102 | + | t.Helper() | |||
| 103 | + | var pid int | |||
| 104 | + | require.Eventually(t, func() bool { | |||
| 105 | + | b, err := os.ReadFile(file) | |||
| 106 | + | if err != nil { | |||
| 107 | + | return false | |||
| 108 | + | } | |||
| 109 | + | pid, err = strconv.Atoi(string(b)) | |||
| 110 | + | return err == nil | |||
| 111 | + | }, time.Second, 10*time.Millisecond) | |||
| 112 | + | return pid | |||
| 113 | + | } | |||
| 114 | + | ||||
| 115 | + | func TestProcDetectorHungChildIsKilledAndRestarted(t *testing.T) { | |||
| 116 | + | for _, mode := range []string{"nohello", "silent"} { | |||
| 117 | + | t.Run(mode, func(t *testing.T) { | |||
| 118 | + | pidFile := t.TempDir() + "/child" | |||
| 119 | + | t.Setenv(helperChildPID, pidFile) | |||
| 120 | + | d := helperDetector(t, mode) | |||
| 121 | + | ctx := context.Background() | |||
| 122 | + | start := time.Now() | |||
| 123 | + | _, err := d.Detect(ctx, tinyFrame()) | |||
| 124 | + | require.ErrorContains(t, err, "timed out") | |||
| 125 | + | if mode == "nohello" { | |||
| 126 | + | require.ErrorContains(t, err, "hello") | |||
| 127 | + | } | |||
| 128 | + | require.Less(t, time.Since(start), 2*time.Second) | |||
| 129 | + | require.Nil(t, d.proc) | |||
| 130 | + | pid := readPID(t, pidFile) | |||
| 131 | + | require.True(t, processGone(pid), "child %d survives", pid) | |||
| 132 | + | ||||
| 133 | + | // The next Detect starts a new, healthy child. | |||
| 134 | + | t.Setenv(helperEnv, "ok") | |||
| 135 | + | t.Setenv(helperChildPID, "") | |||
| 136 | + | _, err = d.Detect(ctx, tinyFrame()) | |||
| 137 | + | require.NoError(t, err) | |||
| 138 | + | require.NotNil(t, d.proc) | |||
| 139 | + | start = time.Now() | |||
| 140 | + | require.NoError(t, d.Close()) | |||
| 141 | + | require.Less(t, time.Since(start), 3*time.Second) | |||
| 142 | + | }) | |||
| 143 | + | } | |||
| 144 | + | } | |||
| 145 | + | ||||
| 146 | + | // A grandchild holding the child's stdout and stderr open must neither | |||
| 147 | + | // survive the kill nor keep Wait blocked. | |||
| 148 | + | func TestProcDetectorKillsGrandchild(t *testing.T) { | |||
| 149 | + | pidFile := t.TempDir() + "/pid" | |||
| 150 | + | t.Setenv(helperPIDFile, pidFile) | |||
| 151 | + | d := helperDetector(t, "grandchild") | |||
| 152 | + | start := time.Now() | |||
| 153 | + | _, err := d.Detect(context.Background(), tinyFrame()) | |||
| 154 | + | require.ErrorContains(t, err, "hello timed out") | |||
| 155 | + | require.Less(t, time.Since(start), 2500*time.Millisecond) | |||
| 156 | + | pid := readPID(t, pidFile) | |||
| 157 | + | require.Eventually(t, func() bool { return processGone(pid) }, 2*time.Second, 20*time.Millisecond, "grandchild %d survives", pid) | |||
| 158 | + | require.Nil(t, d.proc) | |||
| 159 | + | } | |||
| 160 | + | ||||
| 161 | + | // Cancelling the context interrupts a pending read. | |||
| 162 | + | func TestProcDetectorContextCancel(t *testing.T) { | |||
| 163 | + | d := helperDetector(t, "silent") | |||
| 164 | + | d.reqTimeout = time.Minute | |||
| 165 | + | ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond) | |||
| 166 | + | defer cancel() | |||
| 167 | + | start := time.Now() | |||
| 168 | + | _, err := d.Detect(ctx, tinyFrame()) | |||
| 169 | + | require.ErrorIs(t, err, context.DeadlineExceeded) | |||
| 170 | + | require.Less(t, time.Since(start), 2*time.Second) | |||
| 171 | + | require.Nil(t, d.proc) | |||
| 172 | + | } | |||