Skip to content

fix: prevent header times scan panic after stop#3740

Merged
DrJosh9000 merged 3 commits intomainfrom
fix/header-times-scan-after-stop
Mar 12, 2026
Merged

fix: prevent header times scan panic after stop#3740
DrJosh9000 merged 3 commits intomainfrom
fix/header-times-scan-after-stop

Conversation

@lox
Copy link
Contributor

@lox lox commented Mar 6, 2026

Problem

headerTimesStreamer.Stop() closed timesCh but left streaming set to true.

Scan() checks streaming and, when true, sends a timestamp into timesCh. If Scan() runs after Stop() closes the channel, it can panic with:

send on closed channel

Relevant code:

  • stop path: agent/header_times_streamer.go (Stop)
  • send path: agent/header_times_streamer.go (Scan)

Impact

This is a runtime crash risk in job execution cleanup.

In practice, log-processing goroutines can still call Scan() while cleanup is in progress. If that races with Stop(), the agent process can panic instead of finishing job teardown cleanly.

Potential user-visible effects:

  • abrupt agent failure during/after job completion
  • incomplete cleanup and confusing job-end behaviour
  • reduced confidence in log/header-time streaming reliability

Reproduction

Added a regression test that exercises the real lifecycle:

  1. start streamer with Run()
  2. call Stop()
  3. call Scan("--- a header")

Before this fix, the test fails with panic (send on closed channel).

Fix

Set streaming = false under streamingMu before closing timesCh.

This preserves the existing send/close synchronisation while ensuring post-stop Scan() calls do not attempt channel sends.

Testing

  • Added: TestHeaderTimesStreamerScanAfterStopDoesNotPanic
  • Verified with:
    • go test ./agent -run TestHeaderTimesStreamerScanAfterStopDoesNotPanic -count=1

@lox lox requested review from a team as code owners March 6, 2026 05:53
@DrJosh9000 DrJosh9000 enabled auto-merge March 12, 2026 02:34
@DrJosh9000 DrJosh9000 merged commit 6fa6511 into main Mar 12, 2026
2 checks passed
@DrJosh9000 DrJosh9000 deleted the fix/header-times-scan-after-stop branch March 12, 2026 02:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants