Skip to content

Commit ccb0aa7

Browse files
Finish atomic restore-token reset recovery
Ultraworked with [omo](https://github.com/code-yeongyu/oh-my-openagent) Co-authored-by: sisyphus-dev-ai <sisyphus-dev-ai@users.noreply.github.com>
1 parent d9f1625 commit ccb0aa7

3 files changed

Lines changed: 151 additions & 12 deletions

File tree

internal/daemon/daemon.go

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1354,6 +1354,10 @@ func (d *Daemon) getOrStartPreparedCaptureGroup(ctx context.Context, entry *acti
13541354
if d.captureGroups == nil {
13551355
d.captureGroups = make(map[videoCaptureKey]*videoCaptureGroup)
13561356
}
1357+
if _, err := d.migrateRestoreTokenResetReservationLocked(entry, key); err != nil {
1358+
d.mu.Unlock()
1359+
return nil, 0, err
1360+
}
13571361
group := d.captureGroups[key]
13581362
var captureCtx context.Context
13591363
var captureCancel context.CancelFunc
@@ -1474,6 +1478,10 @@ func (d *Daemon) getOrStartCaptureGroup(entry *activeStream, restoreToken, devic
14741478
if d.captureGroups == nil {
14751479
d.captureGroups = make(map[videoCaptureKey]*videoCaptureGroup)
14761480
}
1481+
if _, err := d.migrateRestoreTokenResetReservationLocked(entry, key); err != nil {
1482+
d.mu.Unlock()
1483+
return nil, err
1484+
}
14771485
group := d.captureGroups[key]
14781486
var captureCtx context.Context
14791487
var captureCancel context.CancelFunc
@@ -1565,6 +1573,30 @@ func (d *Daemon) getOrStartCaptureGroup(entry *activeStream, restoreToken, devic
15651573
return newBC, nil
15661574
}
15671575

1576+
// migrateRestoreTokenResetReservationLocked moves an owner-only reset claim
1577+
// when the receiver negotiates a different canvas after reconnecting. It never
1578+
// joins an occupied destination: reset replacement remains exclusive.
1579+
func (d *Daemon) migrateRestoreTokenResetReservationLocked(entry *activeStream, key videoCaptureKey) (*videoCaptureGroup, error) {
1580+
reservation := entry.captureGroup
1581+
if reservation == nil || reservation.resetReservedBy != entry ||
1582+
reservation.broadcast != nil || reservation.capture != nil || reservation.cancel != nil {
1583+
return nil, nil
1584+
}
1585+
if reservation.key == key {
1586+
return reservation, nil
1587+
}
1588+
if d.captureGroups[reservation.key] != reservation {
1589+
return nil, context.Canceled
1590+
}
1591+
if d.captureGroups[key] != nil {
1592+
return nil, fmt.Errorf("%w: replacement canvas %dx%d is already active", errCaptureGroupResetReserved, key.maxWidth, key.maxHeight)
1593+
}
1594+
delete(d.captureGroups, reservation.key)
1595+
reservation.key = key
1596+
d.captureGroups[key] = reservation
1597+
return reservation, nil
1598+
}
1599+
15681600
// detachStreamLocked removes a single stream and transfers ownership of its
15691601
// resources, plus an unused final capture group, to a cleanup plan. The caller
15701602
// must unlock d.mu before running the plan.

internal/daemon/reset_restore_token.go

Lines changed: 26 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,10 @@
11
package daemon
22

3-
import "context"
3+
import (
4+
"context"
5+
6+
"doubletake/internal/airplay"
7+
)
48

59
func (d *Daemon) handleResetRestoreToken(req Request) Response {
610
d.mu.Lock()
@@ -68,22 +72,18 @@ func (d *Daemon) handleResetRestoreToken(req Request) Response {
6872
group.resetReservedBy = nil
6973
}
7074
d.mu.Unlock()
71-
if clearErr == nil {
72-
_ = credentialReset.Rollback()
73-
}
75+
rollbackErr := rollbackRestoreTokenReset(credentialReset, clearErr)
7476
d.streamWorkers.Done()
75-
return Response{OK: false, State: state, Error: "daemon is shutting down"}
77+
return Response{OK: false, State: state, Error: resetFailure("daemon is shutting down", rollbackErr)}
7678
}
7779
if !reservationCurrent {
7880
if group.resetReservedBy == entry {
7981
group.resetReservedBy = nil
8082
}
8183
d.mu.Unlock()
82-
if clearErr == nil {
83-
_ = credentialReset.Rollback()
84-
}
84+
rollbackErr := rollbackRestoreTokenReset(credentialReset, clearErr)
8585
d.streamWorkers.Done()
86-
return Response{OK: false, State: state, Error: "restore token reset was canceled for " + target}
86+
return Response{OK: false, State: state, Error: resetFailure("restore token reset was canceled for "+target, rollbackErr)}
8787
}
8888
if clearErr != nil {
8989
group.resetReservedBy = nil
@@ -132,12 +132,12 @@ func (d *Daemon) handleResetRestoreToken(req Request) Response {
132132
d.mu.Unlock()
133133
abandoned.run()
134134
cancel()
135-
_ = credentialReset.Rollback()
135+
rollbackErr := credentialReset.Rollback()
136136
d.streamWorkers.Done()
137137
if shuttingDown {
138-
return Response{OK: false, State: state, Error: "daemon is shutting down"}
138+
return Response{OK: false, State: state, Error: resetFailure("daemon is shutting down", rollbackErr)}
139139
}
140-
return Response{OK: false, State: state, Error: "restore token reset was canceled for " + target}
140+
return Response{OK: false, State: state, Error: resetFailure("restore token reset was canceled for "+target, rollbackErr)}
141141
}
142142
d.clearLastErrorForTargetLocked(target)
143143
state = d.overallStateLocked()
@@ -151,6 +151,20 @@ func (d *Daemon) handleResetRestoreToken(req Request) Response {
151151
return Response{OK: true, State: state, Device: target, DeviceIP: target}
152152
}
153153

154+
func rollbackRestoreTokenReset(reset *airplay.RestoreTokenReset, clearErr error) error {
155+
if clearErr != nil {
156+
return nil
157+
}
158+
return reset.Rollback()
159+
}
160+
161+
func resetFailure(message string, rollbackErr error) string {
162+
if rollbackErr == nil {
163+
return message
164+
}
165+
return message + "; restore token rollback failed: " + rollbackErr.Error()
166+
}
167+
154168
// restoreTokenResetReservationCurrentLocked reports whether the exact stream
155169
// and exclusive capture generation reserved before credential I/O are still
156170
// current. Must be called with d.mu held.

internal/daemon/reset_restore_token_test.go

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,87 @@ func TestResetRestoreTokenConcurrentDisconnectReportsCancellationAndOverallState
404404
}
405405
}
406406

407+
func TestCanceledResetReportsRollbackPersistenceFailures(t *testing.T) {
408+
for _, test := range []struct {
409+
name string
410+
lookupErrAt int
411+
saveErrAt int
412+
}{
413+
{name: "rollback lookup", lookupErrAt: 2},
414+
{name: "rollback save", saveErrAt: 2},
415+
} {
416+
t.Run(test.name, func(t *testing.T) {
417+
backend := &controlledCredentialBackend{
418+
credentials: &airplay.SavedCredentials{RestoreToken: "restore-1"},
419+
lookupErrAt: test.lookupErrAt,
420+
saveErrAt: test.saveErrAt,
421+
}
422+
d, entry, _, _ := newResetTestDaemon(t, backend)
423+
cleanupStarted := make(chan struct{})
424+
cleanupRelease := make(chan struct{})
425+
originalCancel := entry.cancelFn
426+
entry.cancelFn = func() {
427+
close(cleanupStarted)
428+
<-cleanupRelease
429+
originalCancel()
430+
}
431+
defer d.Shutdown()
432+
433+
resetResponse := make(chan Response, 1)
434+
go func() {
435+
resetResponse <- d.handleResetRestoreToken(Request{Cmd: "reset-restore-token", Target: resetTestTarget})
436+
}()
437+
waitForResetSignal(t, cleanupStarted, "cleanup did not start")
438+
if response := d.handleDisconnect(Request{Cmd: "disconnect", Target: resetTestTarget}); !response.OK {
439+
t.Fatalf("disconnect response = %+v", response)
440+
}
441+
close(cleanupRelease)
442+
443+
response := waitForResetResponse(t, resetResponse)
444+
if response.OK || !strings.Contains(response.Error, "rollback failed") {
445+
t.Fatalf("reset response = %+v, want explicit rollback failure", response)
446+
}
447+
})
448+
}
449+
}
450+
451+
func TestRestoreTokenResetReservationMigratesChangedCanvasExclusively(t *testing.T) {
452+
d, entry, reservation, _ := newResetTestDaemon(t, &controlledCredentialBackend{})
453+
oldKey := reservation.key
454+
newKey := normalizedVideoCaptureKey(1280, 720, airplay.VideoCodecH264)
455+
reservation.broadcast = nil
456+
reservation.resetReservedBy = entry
457+
458+
d.mu.Lock()
459+
migrated, err := d.migrateRestoreTokenResetReservationLocked(entry, newKey)
460+
oldReleased := d.captureGroups[oldKey] == nil
461+
newOwned := d.captureGroups[newKey] == reservation
462+
d.mu.Unlock()
463+
if err != nil || migrated != reservation || !oldReleased || !newOwned {
464+
t.Fatalf("migration = %p, %v, oldReleased=%t newOwned=%t", migrated, err, oldReleased, newOwned)
465+
}
466+
467+
occupiedKey := normalizedVideoCaptureKey(3840, 2160, airplay.VideoCodecH264)
468+
occupied := &videoCaptureGroup{key: occupiedKey}
469+
d.mu.Lock()
470+
d.captureGroups[occupiedKey] = occupied
471+
_, err = d.migrateRestoreTokenResetReservationLocked(entry, occupiedKey)
472+
stillOwned := d.captureGroups[newKey] == reservation && entry.captureGroup == reservation
473+
d.mu.Unlock()
474+
if err == nil || !errors.Is(err, errCaptureGroupResetReserved) || !stillOwned {
475+
t.Fatalf("occupied migration = %v, stillOwned=%t", err, stillOwned)
476+
}
477+
478+
d.mu.Lock()
479+
cleanup := d.detachStreamLocked(resetTestTarget)
480+
reservationReleased := d.captureGroups[newKey] == nil
481+
d.mu.Unlock()
482+
cleanup.run()
483+
if !reservationReleased {
484+
t.Fatal("disconnect left migrated reservation behind")
485+
}
486+
}
487+
407488
func TestResetRestoreTokenClearsExclusiveTargetAndReconnectsActualPort(t *testing.T) {
408489
listener, err := net.Listen("tcp", "127.0.0.1:0")
409490
if err != nil {
@@ -499,9 +580,17 @@ type controlledCredentialBackend struct {
499580
saveStarted chan struct{}
500581
saveRelease <-chan struct{}
501582
saveOnce sync.Once
583+
lookupCalls int
584+
saveCalls int
585+
lookupErrAt int
586+
saveErrAt int
502587
}
503588

504589
func (b *controlledCredentialBackend) Lookup(string) (*airplay.SavedCredentials, error) {
590+
b.lookupCalls++
591+
if b.lookupErrAt > 0 && b.lookupCalls == b.lookupErrAt {
592+
return nil, errors.New("injected lookup failure")
593+
}
505594
if b.lookupErr != nil {
506595
err := b.lookupErr
507596
b.lookupErr = nil
@@ -521,6 +610,10 @@ func (b *controlledCredentialBackend) Save(_ string, credentials *airplay.SavedC
521610
if b.saveRelease != nil {
522611
<-b.saveRelease
523612
}
613+
b.saveCalls++
614+
if b.saveErrAt > 0 && b.saveCalls == b.saveErrAt {
615+
return errors.New("injected save failure")
616+
}
524617
if b.saveErr != nil {
525618
return b.saveErr
526619
}

0 commit comments

Comments
 (0)