Skip to content

Commit ca1996a

Browse files
jameshoulahancuthix
authored andcommitted
fix(GODT-2327): Properly cancel event stream when handling refresh
1 parent ab1c1c4 commit ca1996a

File tree

2 files changed

+39
-24
lines changed

2 files changed

+39
-24
lines changed

internal/user/events.go

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,11 @@ func (user *User) handleRefreshEvent(ctx context.Context, refresh proton.Refresh
8585
l.WithError(err).Error("Failed to report refresh to sentry")
8686
}
8787

88+
// Cancel the event stream once this refresh is done.
89+
defer user.pollAbort.Abort()
90+
8891
// Cancel and restart ongoing syncs.
89-
user.abortable.Abort()
92+
user.syncAbort.Abort()
9093
defer user.goSync()
9194

9295
return safe.LockRet(func() error {

internal/user/user.go

Lines changed: 35 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,8 @@ type User struct {
7878
updateChLock safe.RWMutex
7979

8080
tasks *async.Group
81-
abortable async.Abortable
81+
syncAbort async.Abortable
82+
pollAbort async.Abortable
8283
goSync func()
8384

8485
pollAPIEventsCh chan chan struct{}
@@ -182,29 +183,38 @@ func New(
182183
}
183184
}
184185

185-
// When triggered, attempt to sync the user.
186+
// When triggered, sync the user and then begin streaming API events.
186187
user.goSync = user.tasks.Trigger(func(ctx context.Context) {
187188
user.log.Debug("Sync triggered")
188189

189-
user.abortable.Do(ctx, func(ctx context.Context) {
190-
if !user.vault.SyncStatus().IsComplete() {
191-
if err := user.doSync(ctx); err != nil {
192-
user.log.WithError(err).Error("Failed to sync, will retry later")
193-
194-
go func() {
195-
select {
196-
case <-ctx.Done():
197-
user.log.WithError(err).Warn("Aborting sync retry")
198-
case <-time.After(SyncRetryCooldown):
199-
user.goSync()
200-
}
201-
}()
202-
}
190+
// Sync the user.
191+
user.syncAbort.Do(ctx, func(ctx context.Context) {
192+
if user.vault.SyncStatus().IsComplete() {
193+
user.log.Debug("Sync already complete, skipping")
194+
return
195+
}
196+
197+
if err := user.doSync(ctx); err != nil {
198+
user.log.WithError(err).Error("Failed to sync, will retry later")
199+
200+
go func() {
201+
select {
202+
case <-ctx.Done():
203+
user.log.WithError(err).Warn("Aborting sync retry")
204+
case <-time.After(SyncRetryCooldown):
205+
user.goSync()
206+
}
207+
}()
203208
}
204209

205-
// Once we know the sync has completed, we can start polling for API events.
206-
user.startEvents(ctx)
207210
})
211+
212+
// Once we know the sync has completed, we can start polling for API events.
213+
if user.vault.SyncStatus().IsComplete() {
214+
user.pollAbort.Do(ctx, func(ctx context.Context) {
215+
user.startEvents(ctx)
216+
})
217+
}
208218
})
209219

210220
return user, nil
@@ -270,7 +280,8 @@ func (user *User) GetAddressMode() vault.AddressMode {
270280
func (user *User) SetAddressMode(_ context.Context, mode vault.AddressMode) error {
271281
user.log.WithField("mode", mode).Info("Setting address mode")
272282

273-
user.abortable.Abort()
283+
user.syncAbort.Abort()
284+
user.pollAbort.Abort()
274285
defer user.goSync()
275286

276287
return safe.LockRet(func() error {
@@ -461,7 +472,8 @@ func (user *User) OnStatusUp(context.Context) {
461472
func (user *User) OnStatusDown(context.Context) {
462473
user.log.Info("Connection is down")
463474

464-
user.abortable.Abort()
475+
user.syncAbort.Abort()
476+
user.pollAbort.Abort()
465477
}
466478

467479
// GetSyncStatus returns the sync status of the user.
@@ -471,6 +483,7 @@ func (user *User) GetSyncStatus() vault.SyncStatus {
471483

472484
// ClearSyncStatus clears the sync status of the user.
473485
// This also drops any updates in the update channel(s).
486+
// Warning: the gluon user must be removed and re-added if this happens!
474487
func (user *User) ClearSyncStatus() error {
475488
user.log.Info("Clearing sync status")
476489

@@ -481,6 +494,7 @@ func (user *User) ClearSyncStatus() error {
481494

482495
// clearSyncStatus clears the sync status of the user.
483496
// This also drops any updates in the update channel(s).
497+
// Warning: the gluon user must be removed and re-added if this happens!
484498
// It is assumed that the eventLock, apiAddrsLock and updateChLock are already locked.
485499
func (user *User) clearSyncStatus() error {
486500
user.log.Info("Clearing sync status")
@@ -593,9 +607,7 @@ func (user *User) startEvents(ctx context.Context) {
593607

594608
user.log.Debug("Event poll triggered")
595609

596-
if !user.vault.SyncStatus().IsComplete() {
597-
user.log.Debug("Sync is incomplete, skipping event poll")
598-
} else if err := user.doEventPoll(ctx); err != nil {
610+
if err := user.doEventPoll(ctx); err != nil {
599611
user.log.WithError(err).Error("Failed to poll events")
600612
}
601613

0 commit comments

Comments
 (0)