v2/secret: RotatingFileProvider.Shutdown can panic with "send on closed channel"

Note

⚠️ This issue description is AI generated.

The issue was identified with my two human eyes. I've reviewed the description thoroughly.

Summary

RotatingFileProvider.Shutdown closes the notify channel from the caller's goroutine while the poll goroutine, which is the channel's only sender, may still be running. A send on a closed channel panics, and nothing recovers it, so the process crashes during graceful shutdown.

Where

v2/secret/rotating_file.go

  • :102 close(p.notify) in Shutdown
  • :140, :154 the only sends, both in the poll goroutine

Why it happens

Shutdown waits for the goroutine to exit or for its context to expire, whichever comes first, then closes notify on both paths:

select {
case <-stopped:      // goroutine exited: safe
case <-ctx.Done():
    err = ctx.Err()  // goroutine still running
}
close(p.notify)      // :102

On the ctx.Done() path the goroutine still owns the channel. The guard at :129 (if ctx.Err() != nil { return }) narrows the window but cannot close it, since it is a check-then-act on state another goroutine mutates:

  1. goroutine checks ctx.Err(), sees nil, falls through
  2. goroutine is descheduled
  3. Shutdown takes the ctx.Done() branch and closes notify
  4. goroutine resumes, sends at :154, panics

Note the sends use select { case p.notify <- event: default: }. The default guards a full buffer, not a closed channel, so it offers no protection here.

How it is reached

Shutdown's context must expire before the goroutine exits, which is the normal app.App graceful-shutdown path under a deadline. FileProvider.Get reads via os.ReadFile (v2/secret/file.go:63) and ignores its context, so a slow or hung mount parks the goroutine inside Get for as long as the kernel takes. The deadline fires, Shutdown closes notify, and the send panics when the read final recover in this path. A service shutting down cleanly under a tight deadline crashes instead, which is likely to be misattributed to anything but secret rotation.

Proposed fix

Return the channel to its sender. poll closes it as it exits:

func (p *RotatingFileProvider) poll(ctx context.Context, stopped chan struct{}) {
      // poll is the only sender on notify, so it closes the channel as it
      // exits; closing it from Shutdown could race a send in progress here.
      defer close(stopped)
      defer close(p.notify)

Defers run LIFO, so notify closes before stopped, meaning once Shutdown observes stopped closed, notify is closed too. Shutdown then drops its close(p.notify), and stopOnce loses its only purpose and can go with it, leaving Shutdown naturally idempotent.

Acceptance criteria (If Proposed Fix is accepted)

  • poll closes notify; Shutdown does not
  • stopOnce removed, Shutdown still idempotent
  • Regression test: after Shutdown returns ctx.Err(), notify is still open. Deterministic via a non-blocking receive, where ok == false means it was wrongly closed
  • Existing v2/secret tests pass under -race
  • Doc comment updated: OnRotate's channel is closed by the poll goroutine on exit, not by Shutdown

Behaviour change to call out in the MR

On the deadline path notify now closes later, when the goroutine actually finishes, rather than immediately. A consumer using for event := range p.OnRotate() (the pattern in v2/secret/README.md:79) blocks marginally longer in that case. Preferable to crashing.

Related

Found while investigating a flaky test in the same function: !707 (merged)