diff --git a/internal/updates/sse.go b/internal/updates/sse.go index 700790efb8..8f146003bd 100644 --- a/internal/updates/sse.go +++ b/internal/updates/sse.go @@ -105,7 +105,11 @@ func (b *SSEBroadcaster) AddClient(w http.ResponseWriter, clientID string) *SSEC // before the ordered writer starts, so a (re)connecting client always // sees where the update is right now instead of waiting for the next // stage change. The cache is read after registration so a broadcast - // racing this connect is either in the snapshot or queued behind it. + // racing this connect is never lost: it is in the snapshot, queued + // behind it, or both. In the last case the client receives that status + // again after the snapshot, possibly preceded by an older stage that was + // still in flight. Stage progress only rises, and the update modal + // ignores stages that move it backwards, so the repeat is harmless. b.statusMu.RLock() cachedStatus := b.cachedStatus b.statusMu.RUnlock() diff --git a/internal/updates/sse_test.go b/internal/updates/sse_test.go index 15da113887..4e97fb6fc8 100644 --- a/internal/updates/sse_test.go +++ b/internal/updates/sse_test.go @@ -249,7 +249,23 @@ func TestSSEBroadcaster_AddClientWritesCurrentStatusBeforeReturning(t *testing.T broadcaster := NewSSEBroadcaster() defer broadcaster.Close() + // The stage was broadcast before this client connects, so let it reach + // an existing client first. While a broadcast is still in flight the + // broadcast loop can queue it for the new client as well, and that + // client's writer goroutine would then write the recorder concurrently + // with the unsynchronized reads below. + earlier := &syncFlushWriter{} + if client := broadcaster.AddClient(earlier, "client-earlier"); client == nil { + t.Fatal("AddClient returned nil") + } broadcaster.Broadcast(UpdateStatus{Status: "extracting", Progress: 40, Message: "Extracting update..."}) + deadline := time.Now().Add(2 * time.Second) + for !strings.Contains(earlier.String(), `"status":"extracting"`) { + if time.Now().After(deadline) { + t.Fatalf("broadcast never reached the earlier client: %q", earlier.String()) + } + time.Sleep(time.Millisecond) + } w := &mockFlushWriter{ResponseRecorder: httptest.NewRecorder()} if client := broadcaster.AddClient(w, "client-now"); client == nil {