Skip to content

Fix runstream subscriber close race - #244

Merged
MiniMax-AI-Dev merged 1 commit into
MiniMax-AI-Dev:mainfrom
NianJiuZst:codex/fix-runstream-broker-race
Aug 11, 2026
Merged

Fix runstream subscriber close race#244
MiniMax-AI-Dev merged 1 commit into
MiniMax-AI-Dev:mainfrom
NianJiuZst:codex/fix-runstream-broker-race

Conversation

@NianJiuZst

Copy link
Copy Markdown
Contributor

Summary

  • serialize runstream subscriber sends and closes under the broker mutex
  • replay buffered events before registering live subscribers
  • add a regression for concurrent SSE cancellation and event publication

Root cause and impact

Publish previously snapshotted subscriber channels under the mutex, released the lock, and then sent to them. An SSE request cancellation could concurrently unsubscribe and close one of those channels, causing Publish to panic with send on closed channel. Because the panic occurs in a server goroutine without recovery, a single disconnected client could terminate the service process.

The broker now keeps subscriber sends, removals, and closes in one synchronization domain, so a channel cannot be closed while a publisher or replay is sending to it.

Validation

  • GOMAXPROCS=4 go test ./server/internal/runstream -run '^TestPublishConcurrentWithCancelDoesNotPanic$' -count=20
  • go test ./server/internal/runstream -count=10
  • go test -race ./server/internal/runstream -count=1
  • go test ./server/internal/dev -run 'RunStream|ConversationRun|StartConversation' -count=1
  • make check

make check passed. Docker was unavailable locally, so the PostgreSQL migration smoke test was skipped; this change does not touch database code or migrations.

@NianJiuZst
NianJiuZst marked this pull request as ready for review August 10, 2026 13:19
@MiniMax-AI-Dev
MiniMax-AI-Dev merged commit 90146d4 into MiniMax-AI-Dev:main Aug 11, 2026
13 checks passed
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.

2 participants