mirror of
https://github.com/gesellix/Bose-SoundTouch.git
synced 2026-09-07 15:07:17 +00:00
fix(player): stamp a connection epoch so revisions stay comparable
DeviceStatus.Revision is per-connection and restarts at 0. The browser compares revisions to decide which frame wins, but nothing in the frame said which revision sequence it belonged to. So a device id backed by a fresh DeviceConnection published revisions starting at 0 while an open tab still held a high revision for that id, and the tab rejected every later frame for it: a status frozen until reload. HandleDeleteDevice broadcasts after removal, which covers the ordinary remove-then-rediscover path, but a discovery sweep re-adding the host inside that window yields a snapshot that already contains the device at revision 0, so no device-less snapshot is ever sent. Every status now carries an Epoch identifying the connection that produced it, stamped by both SetStatus and UpdateStatus. The browser compares epochs first and only falls back to revisions within one epoch, so a newer connection is accepted regardless of its revision and a frame still in flight from the replaced connection is rejected regardless of its. nextStatusEpoch is seeded from the wall clock and forced strictly increasing, so epochs also keep rising across a service restart, where a plain counter would restart at 0 and reintroduce the same problem. It is in milliseconds because the browser compares it as a JSON number and a nanosecond timestamp exceeds Number.MAX_SAFE_INTEGER. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
4fe5ad50b4
commit
eec8975633
@@ -246,6 +246,42 @@ window.revisionChecks = {
|
||||
sourcesStale: false,
|
||||
}) === stale,
|
||||
unknownRejected: mergeStatusUpdate(newer, 'unknown', { revision: 99 }) === newer,
|
||||
// A fresh DeviceConnection for the same id restarts revisions at 0. Without
|
||||
// the epoch check this frame loses the revision comparison and the tab stays
|
||||
// pinned to the old status forever.
|
||||
newerEpochAcceptedDespiteLowerRevision: (() => {
|
||||
const epoched = mergeStatusUpdate(newer, 'speaker', {
|
||||
epoch: 100,
|
||||
revision: 40,
|
||||
nowPlaying: { Track: 'epoch one' },
|
||||
});
|
||||
const reconnected = mergeStatusUpdate(epoched, 'speaker', {
|
||||
epoch: 101,
|
||||
revision: 0,
|
||||
nowPlaying: { Track: 'epoch two' },
|
||||
});
|
||||
return reconnected !== epoched &&
|
||||
reconnected.speaker.status.nowPlaying.Track === 'epoch two';
|
||||
})(),
|
||||
olderEpochRejected: (() => {
|
||||
const epochOne = mergeStatusUpdate(newer, 'speaker', {
|
||||
epoch: 100,
|
||||
revision: 40,
|
||||
nowPlaying: { Track: 'epoch one' },
|
||||
});
|
||||
const epochTwo = mergeStatusUpdate(epochOne, 'speaker', {
|
||||
epoch: 101,
|
||||
revision: 0,
|
||||
nowPlaying: { Track: 'epoch two' },
|
||||
});
|
||||
// A frame still in flight from the replaced connection, carrying a high
|
||||
// revision from its own sequence, must not win.
|
||||
return mergeStatusUpdate(epochTwo, 'speaker', {
|
||||
epoch: 100,
|
||||
revision: 99,
|
||||
nowPlaying: { Track: 'late frame from the old connection' },
|
||||
}) === epochTwo;
|
||||
})(),
|
||||
adversarialIDsAreOwnDataProperties:
|
||||
Object.prototype.hasOwnProperty.call(constructorUpdated, '__proto__') &&
|
||||
Object.prototype.hasOwnProperty.call(constructorUpdated, 'constructor'),
|
||||
|
||||
@@ -26,11 +26,27 @@ function statusRevision(status) {
|
||||
return Number.isSafeInteger(revision) && revision >= 0 ? revision : null;
|
||||
}
|
||||
|
||||
function statusEpoch(status) {
|
||||
const epoch = status?.epoch;
|
||||
return Number.isSafeInteger(epoch) ? epoch : null;
|
||||
}
|
||||
|
||||
// The server advances DeviceStatus.revision on every projection, so a frame
|
||||
// carrying a revision no newer than what we already hold is stale -- a slow
|
||||
// `devices` snapshot overtaken by a `status_update` delta, or a REST refresh
|
||||
// overtaken by either. A device we have never seen is always accepted.
|
||||
//
|
||||
// Revisions are only comparable within one epoch. A device id backed by a new
|
||||
// DeviceConnection, or a restarted service, restarts its revisions at 0, and
|
||||
// without the epoch check the browser would reject every later frame for that
|
||||
// id and display a status frozen at whatever it last held.
|
||||
function acceptsNewerStatus(current, incoming) {
|
||||
const currentEpoch = statusEpoch(current);
|
||||
const incomingEpoch = statusEpoch(incoming);
|
||||
if (currentEpoch !== null && incomingEpoch !== null && incomingEpoch !== currentEpoch) {
|
||||
return incomingEpoch > currentEpoch;
|
||||
}
|
||||
|
||||
const currentRevision = statusRevision(current);
|
||||
const incomingRevision = statusRevision(incoming);
|
||||
if (currentRevision === null) return true;
|
||||
|
||||
@@ -95,6 +95,46 @@ func TestNowPlayingRevisionAdvancesForPollAndEvent(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestStatusEpochDistinguishesConnections: Revision restarts at 0 for every
|
||||
// new DeviceConnection, so a browser holding a high revision for a device id
|
||||
// would reject the replacement connection's frames forever. Epoch is what
|
||||
// makes the two sequences distinguishable, and it must strictly increase.
|
||||
func TestStatusEpochDistinguishesConnections(t *testing.T) {
|
||||
first := NewDeviceConnection(nil, nil)
|
||||
second := NewDeviceConnection(nil, nil)
|
||||
|
||||
firstEpoch := first.Status().Epoch
|
||||
secondEpoch := second.Status().Epoch
|
||||
|
||||
if firstEpoch <= 0 || secondEpoch <= firstEpoch {
|
||||
t.Fatalf("epochs = %d then %d, want strictly increasing and non-zero", firstEpoch, secondEpoch)
|
||||
}
|
||||
|
||||
// Both entry points must stamp it: a status that lost its epoch would be
|
||||
// indistinguishable from one produced before epochs existed.
|
||||
first.UpdateStatus(func(status *DeviceStatus) {
|
||||
status.NowPlaying = &models.NowPlaying{Source: "AUX"}
|
||||
})
|
||||
if got := first.Status().Epoch; got != firstEpoch {
|
||||
t.Errorf("epoch after UpdateStatus = %d, want %d", got, firstEpoch)
|
||||
}
|
||||
|
||||
first.SetStatus(&DeviceStatus{})
|
||||
if got := first.Status().Epoch; got != firstEpoch {
|
||||
t.Errorf("epoch after SetStatus = %d, want %d", got, firstEpoch)
|
||||
}
|
||||
}
|
||||
|
||||
// TestStatusEpochStaysWithinJavaScriptSafeIntegers: the browser compares this
|
||||
// value as a JSON number, so it must not exceed Number.MAX_SAFE_INTEGER.
|
||||
func TestStatusEpochStaysWithinJavaScriptSafeIntegers(t *testing.T) {
|
||||
const maxSafeInteger = int64(1)<<53 - 1
|
||||
|
||||
if got := NewDeviceConnection(nil, nil).Status().Epoch; got > maxSafeInteger {
|
||||
t.Fatalf("epoch = %d, exceeds Number.MAX_SAFE_INTEGER (%d)", got, maxSafeInteger)
|
||||
}
|
||||
}
|
||||
|
||||
func TestDeviceStatusRevisionIsMonotonicWithConcurrentProjections(t *testing.T) {
|
||||
conn := NewDeviceConnection(nil, nil)
|
||||
const projections = 64
|
||||
|
||||
@@ -54,6 +54,9 @@ type DeviceConnection struct {
|
||||
deviceName atomic.Pointer[string]
|
||||
status atomic.Pointer[DeviceStatus]
|
||||
|
||||
// epoch stamps every status this connection publishes. See nextStatusEpoch.
|
||||
epoch int64
|
||||
|
||||
webSocketMu sync.RWMutex
|
||||
webSocketLoopRunning atomic.Bool
|
||||
|
||||
@@ -121,6 +124,10 @@ type DeviceStatus struct {
|
||||
IsConnected bool `json:"isConnected"`
|
||||
LastActivity time.Time `json:"lastActivity"`
|
||||
|
||||
// Epoch identifies the DeviceConnection that produced this status.
|
||||
// Revision restarts at 0 for every new connection, so revisions from
|
||||
// different epochs are not comparable; a client must compare Epoch first.
|
||||
Epoch int64 `json:"epoch"`
|
||||
// Revision is a per-connection monotonic counter advanced by every
|
||||
// successful UpdateStatus. It lets the browser order a full `devices`
|
||||
// snapshot against a `status_update` delta -- without it the two frames
|
||||
@@ -157,6 +164,33 @@ type SpeakerConnectionState struct {
|
||||
Signal string `json:"signal,omitempty"`
|
||||
}
|
||||
|
||||
// statusEpochClock hands out strictly increasing epochs, seeded from the wall
|
||||
// clock so they keep increasing across a service restart too. A plain counter
|
||||
// would restart at 0 on restart and a plain timestamp could collide for two
|
||||
// connections created in the same millisecond; both would leave a browser
|
||||
// unable to tell a newer connection's revision sequence from an older one.
|
||||
var statusEpochClock atomic.Int64
|
||||
|
||||
func nextStatusEpoch() int64 {
|
||||
// Milliseconds, not nanoseconds: this value is compared in the browser,
|
||||
// where a nanosecond timestamp exceeds Number.MAX_SAFE_INTEGER and would
|
||||
// lose precision as a JSON number.
|
||||
now := time.Now().UnixMilli()
|
||||
|
||||
for {
|
||||
previous := statusEpochClock.Load()
|
||||
|
||||
next := now
|
||||
if next <= previous {
|
||||
next = previous + 1
|
||||
}
|
||||
|
||||
if statusEpochClock.CompareAndSwap(previous, next) {
|
||||
return next
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// StatusField identifies one independently-racing field of DeviceStatus for
|
||||
// BeginFieldPoll/CompleteFieldPoll/ApplyFieldEvent's generation ordering.
|
||||
// FieldConnectivity covers IsConnected, which is derived by a poll (from
|
||||
@@ -187,11 +221,13 @@ func NewDeviceConnection(c *client.Client, info *models.DeviceInfo) *DeviceConne
|
||||
DeviceInfo: info,
|
||||
LastSeen: time.Now(),
|
||||
done: make(chan struct{}),
|
||||
epoch: nextStatusEpoch(),
|
||||
}
|
||||
conn.status.Store(&DeviceStatus{
|
||||
Connectivity: ConnectivityOffline,
|
||||
IsConnected: false,
|
||||
LastActivity: time.Now(),
|
||||
Epoch: conn.epoch,
|
||||
})
|
||||
|
||||
if info != nil {
|
||||
@@ -360,6 +396,7 @@ func (c *DeviceConnection) SetStatus(s *DeviceStatus) {
|
||||
for {
|
||||
old := c.status.Load()
|
||||
next := *s
|
||||
next.Epoch = c.epoch
|
||||
next.Revision = old.Revision + 1
|
||||
next.NowPlayingRevision = nowPlayingGeneration
|
||||
|
||||
@@ -468,6 +505,7 @@ func (c *DeviceConnection) UpdateStatus(mut func(*DeviceStatus)) {
|
||||
old := c.status.Load()
|
||||
next := *old
|
||||
mut(&next)
|
||||
next.Epoch = c.epoch
|
||||
next.Revision = old.Revision + 1
|
||||
|
||||
if c.status.CompareAndSwap(old, &next) {
|
||||
|
||||
Reference in New Issue
Block a user