From 5d067ee82d7c0446efdd1ae2cffa40e208ec2843 Mon Sep 17 00:00:00 2001 From: Hermes Date: Fri, 6 Nov 2026 16:37:39 +0100 Subject: [PATCH] fix: bound the event drain so the OSD timer punch survives storms (#3) --- internal/display/x11.go | 17 +++++- internal/display/x11_test.go | 100 +++++++++++++++++++++++++++++++++++ 2 files changed, 116 insertions(+), 1 deletion(-) diff --git a/internal/display/x11.go b/internal/display/x11.go index 7cee954..251fa84 100644 --- a/internal/display/x11.go +++ b/internal/display/x11.go @@ -206,6 +206,16 @@ func createWindowValues(backPixel uint32, eventMask uint32, cursor xproto.Cursor // only as a downstream BadWindow. const cursorBitmapDepth = 1 +// eventDrainBatch bounds how many pending events one wait cycle drains. A +// busy server (video fullscreen repaints) can serve unrelated non-Expose +// events faster than they arrive; an unbounded drain would never reach the +// timer checks, so the OSD overlay's initially empty bounding shape would +// never be punched and its telemetry would never appear. Thirty-two events +// per 100 ms poll cycle sustains well over three thousand events per +// second — orders of magnitude above any plausible root-event rate — while +// bounding liveness and cancellation latency to a single batch. +const eventDrainBatch = 32 + // blankCursorX is the checked X-request surface of the blank cursor setup, // so the sequence can be driven by tests that script failures at exact // operations and pin request order and resource reclamation. @@ -956,7 +966,12 @@ func (s *x11Session) WaitEvent(ctx context.Context) (bool, error) { defer ticker.Stop() lastRefresh := time.Now() for { - for { + // Drain a bounded batch of pending events: a busy server (video + // fullscreen repaints) can serve unrelated non-Expose events faster + // than they arrive, and an unbounded drain would never reach the + // timer checks — the OSD overlay's initially empty bounding shape + // would never be punched and its telemetry would never appear. + for drained := 0; drained < eventDrainBatch; drained++ { event, err := s.pollEvent() if err != nil { return false, err diff --git a/internal/display/x11_test.go b/internal/display/x11_test.go index 58fd132..bd0b778 100644 --- a/internal/display/x11_test.go +++ b/internal/display/x11_test.go @@ -6,6 +6,7 @@ import ( "fmt" "strconv" "strings" + "sync/atomic" "testing" "time" @@ -512,6 +513,105 @@ func TestTransparentShapesEmptyInputAndOutput(t *testing.T) { } } +// stormSession builds an OSD layer session whose event source never runs +// dry: it serves an endless stream of non-Expose root events (the video +// fullscreen repaint storm), so the drain loop can only exit through its +// bound — never through an empty queue. +func stormSession(refresh time.Duration) *x11Session { + return &x11Session{ + poll: time.Millisecond, + refresh: refresh, + pollEvent: func() (xgb.Event, xgb.Error) { + return xproto.ConfigureNotifyEvent{}, nil + }, + probe: func() error { return nil }, + } +} + +func TestWaitEventRefreshesDuringContinuousEventStorm(t *testing.T) { + // The initially empty bounding shape is punched only by redraws: an + // endless stream of unrelated events must never starve the timer that + // drives the OSD refresh cadence. + session := stormSession(3 * time.Millisecond) + session.topVideo = 0 + + done := make(chan eventResult, 1) + go func() { + redraw, err := session.WaitEvent(context.Background()) + done <- eventResult{redraw: redraw, err: err} + }() + select { + case result := <-done: + if result.err != nil { + t.Fatalf("WaitEvent=%v", result.err) + } + if !result.redraw { + t.Fatal("the refresh must request a redraw during an event storm") + } + case <-time.After(2 * time.Second): + t.Fatal("WaitEvent starved: the timer redraw never fired during the storm") + } +} + +func TestWaitEventHonorsCancellationDuringEventStorm(t *testing.T) { + session := stormSession(time.Hour) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + done := make(chan eventResult, 1) + go func() { + redraw, err := session.WaitEvent(ctx) + done <- eventResult{redraw: redraw, err: err} + }() + select { + case result := <-done: + if result.redraw { + t.Fatal("cancellation must not be reported as a redraw") + } + if !errors.Is(result.err, context.Canceled) { + t.Fatalf("err=%v want clean cancellation during the storm", result.err) + } + case <-time.After(2 * time.Second): + t.Fatal("WaitEvent did not terminate on cancellation during the storm") + } +} + +func TestWaitEventStillRedrawsOnExposeDuringEventStorm(t *testing.T) { + var served atomic.Bool + session := stormSession(0) + session.pollEvent = func() (xgb.Event, xgb.Error) { + if served.CompareAndSwap(false, true) { + return xproto.ExposeEvent{}, nil + } + return xproto.ConfigureNotifyEvent{}, nil + } + + redraw, err := session.WaitEvent(context.Background()) + if err != nil { + t.Fatal(err) + } + if !redraw { + t.Fatal("an expose event must still trigger a redraw during a storm") + } +} + +func TestRoleBelowCanvasStaysEventDrivenDuringEventStorm(t *testing.T) { + // The fallback canvas has no refresh interval: an endless unrelated + // event stream must never produce refresh redraws, and only + // cancellation ends the wait. + session := stormSession(0) + + ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) + defer cancel() + redraw, err := session.WaitEvent(ctx) + if !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("err=%v want clean cancellation without a refresh interval", err) + } + if redraw { + t.Fatal("the canvas must stay event-driven with no refresh redraws") + } +} + func TestOSDTextRectCoversTheGlyphCellAroundTheBaseline(t *testing.T) { rect := osdTextRect(32, 32, 9, 6, 11, 3) if rect.X != 30 || rect.Y != 19 {