From 342cd47e6a2a083c215f61aeea743dcbdce9d197 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luk=C3=A1=C5=A1=20Lipinsk=C3=BD?= <6032558+Mr-Tao@users.noreply.github.com> Date: Sat, 29 Aug 2026 21:24:49 +0200 Subject: [PATCH] fix(player): snapshot device timestamps safely --- .../soundtouchweb/device_projection.go | 2 +- .../soundtouchweb/device_projection_test.go | 47 ++++++++++++++++++- pkg/service/soundtouchweb/handler.go | 21 +++++---- 3 files changed, 60 insertions(+), 10 deletions(-) diff --git a/pkg/service/soundtouchweb/device_projection.go b/pkg/service/soundtouchweb/device_projection.go index e5e28b8f..332b8b4a 100644 --- a/pkg/service/soundtouchweb/device_projection.go +++ b/pkg/service/soundtouchweb/device_projection.go @@ -83,7 +83,7 @@ func captureDeviceProjectionEntries(snapshot []DeviceEntry) []deviceProjectionEn ID: entry.ID, Info: entry.Device.DeviceInfo, Status: entry.Device.Status(), - LastSeen: entry.Device.LastSeen, + LastSeen: entry.LastSeen, }) } diff --git a/pkg/service/soundtouchweb/device_projection_test.go b/pkg/service/soundtouchweb/device_projection_test.go index 2989db30..2a9c0b30 100644 --- a/pkg/service/soundtouchweb/device_projection_test.go +++ b/pkg/service/soundtouchweb/device_projection_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "net/http" "net/http/httptest" + "sync" "testing" "time" @@ -19,7 +20,7 @@ func projectionDevice(host, deviceID, name string, connected bool, group *models }) conn.SetStatus(&webtypes.DeviceStatus{IsConnected: connected, Group: group}) - return DeviceEntry{ID: host, Device: conn} + return DeviceEntry{ID: host, Device: conn, LastSeen: conn.LastSeen} } func testStereoGroup() *models.Group { @@ -173,6 +174,50 @@ func TestProjectCapturedDeviceEntriesUsesOneCoherentStatusPerDevice(t *testing.T } } +func TestDeviceViewSnapshotConcurrentTouchUsesCapturedLastSeen(t *testing.T) { + app := NewWebApp() + conn := newRegistryDevice("Living Room") + if !app.AddDevice("192.0.2.10", conn) { + t.Fatal("AddDevice returned false on first insert") + } + + stale := app.DeviceSnapshot() + if len(stale) != 1 { + t.Fatalf("DeviceSnapshot len = %d, want 1", len(stale)) + } + + if !app.TouchDevice("192.0.2.10") { + t.Fatal("TouchDevice returned false for registered device") + } + if got := projectDeviceEntries(stale)["192.0.2.10"].LastSeen; got != stale[0].LastSeen { + t.Fatalf("projection LastSeen = %s, want captured value %s", got, stale[0].LastSeen) + } + + const iterations = 1000 + start := make(chan struct{}) + var wg sync.WaitGroup + wg.Add(2) + + go func() { + defer wg.Done() + <-start + for i := 0; i < iterations; i++ { + app.TouchDevice("192.0.2.10") + } + }() + + go func() { + defer wg.Done() + <-start + for i := 0; i < iterations; i++ { + _ = app.deviceViewSnapshot() + } + }() + + close(start) + wg.Wait() +} + func TestHandleAPIDevicesUsesLogicalStereoProjection(t *testing.T) { app := NewWebApp() group := testStereoGroup() diff --git a/pkg/service/soundtouchweb/handler.go b/pkg/service/soundtouchweb/handler.go index 34401063..222e07c4 100644 --- a/pkg/service/soundtouchweb/handler.go +++ b/pkg/service/soundtouchweb/handler.go @@ -114,11 +114,12 @@ func (app *WebApp) proxyServiceURL() string { return app.ServiceURL } -// DeviceEntry pairs a device id with its connection. Used by -// DeviceSnapshot so callers can iterate without holding the lock. +// DeviceEntry pairs a device id with its connection and the LastSeen value +// captured by DeviceSnapshot under the registry lock. type DeviceEntry struct { - ID string - Device *webtypes.DeviceConnection + ID string + Device *webtypes.DeviceConnection + LastSeen time.Time } // NewWebApp creates a new WebApp instance for SPA mode @@ -142,9 +143,9 @@ func (app *WebApp) GetDevice(id string) (*webtypes.DeviceConnection, bool) { return device, ok } -// DeviceSnapshot returns a list of (id, *DeviceConnection) pairs taken -// under a single read lock. Callers can iterate the result without -// holding any registry lock. Devices added or removed after the call +// DeviceSnapshot returns device entries taken under a single read lock. +// Callers can iterate the result without holding any registry lock. +// Devices added or removed after the call // are not reflected. A pointer captured here stays valid even if the // device is later removed (RemoveDevice only detaches it from the map // and stops its goroutines), so iterating a stale snapshot is safe. @@ -154,7 +155,11 @@ func (app *WebApp) DeviceSnapshot() []DeviceEntry { out := make([]DeviceEntry, 0, len(app.devices)) for id, device := range app.devices { - out = append(out, DeviceEntry{ID: id, Device: device}) + out = append(out, DeviceEntry{ + ID: id, + Device: device, + LastSeen: device.LastSeen, + }) } return out