docs(syncdispatch): clarify timeout state transitions
- commit
- 4fbce1b364ba423c75a8f813bc6af56a5a730636
- author
- brice · · 2026-06-03 05:17 UTC
- parent
- 8bbb568
- signature
- GPG Verified
CD88F9E564E0102646FB6077A1A07C7C7C3E349D· arch-gpg Verified for the committer email
2 files · +9 -0
M internal/infrastructure/snapshot/service.go +2 -0
| @@ -172,6 +172,8 @@ func (s *Service) saveSnapshot(ctx context.Context) error { | |||
| 172 | 172 | } | |
| 173 | 173 | ||
| 174 | 174 | windows, activeWindowIndex := s.provider.GetWindowSnapshotState() | |
| 175 | + | // A nil window list with no active index means the snapshot is truly unavailable, | |
| 176 | + | // not merely empty. Keep the session dirty so a later capture can retry. | |
| 175 | 177 | if windows == nil && activeWindowIndex < 0 { | |
| 176 | 178 | s.markDirty() | |
| 177 | 179 | logging.FromContext(ctx).Warn().Msg("window snapshot unavailable; keeping session snapshot dirty") | |
| @@ -172,6 +172,8 @@ func (s *Service) saveSnapshot(ctx context.Context) error { | |||||
| 172 | } | 172 | } | ||
| 173 | 173 | ||||
| 174 | windows, activeWindowIndex := s.provider.GetWindowSnapshotState() | 174 | windows, activeWindowIndex := s.provider.GetWindowSnapshotState() | ||
| 175 | + | // A nil window list with no active index means the snapshot is truly unavailable, | |||
| 176 | + | // not merely empty. Keep the session dirty so a later capture can retry. | |||
| 175 | if windows == nil && activeWindowIndex < 0 { | 177 | if windows == nil && activeWindowIndex < 0 { | ||
| 176 | s.markDirty() | 178 | s.markDirty() | ||
| 177 | logging.FromContext(ctx).Warn().Msg("window snapshot unavailable; keeping session snapshot dirty") | 179 | logging.FromContext(ctx).Warn().Msg("window snapshot unavailable; keeping session snapshot dirty") | ||
M internal/shared/syncdispatch/sync_dispatch.go +7 -0
| @@ -122,6 +122,9 @@ func RunSynchronousDispatch(opts SyncDispatchOptions, fn func()) SyncDispatchRes | |||
| 122 | 122 | if state != dispatchStateInit { | |
| 123 | 123 | return false | |
| 124 | 124 | } | |
| 125 | + | // Check deadline and transition to dispatchStateStarted under one lock so | |
| 126 | + | // the timer cannot mark dispatchStateTimedOut after this callback is allowed | |
| 127 | + | // to start. Late-start cleanup opts out through AllowLateStartAfterTimeout. | |
| 125 | 128 | if !opts.AllowLateStartAfterTimeout && time.Now().After(deadline) { | |
| 126 | 129 | state = dispatchStateTimedOut | |
| 127 | 130 | return false | |
| @@ -159,6 +162,8 @@ func RunSynchronousDispatch(opts SyncDispatchOptions, fn func()) SyncDispatchRes | |||
| 159 | 162 | stateMu.Lock() | |
| 160 | 163 | currentState := state | |
| 161 | 164 | if opts.AllowLateStartAfterTimeout && currentState == dispatchStateInit { | |
| 165 | + | // Cleanup work may remain queued after timer expiry; decision callbacks | |
| 166 | + | // instead fall through and mark dispatchStateTimedOut while still unstarted. | |
| 162 | 167 | stateMu.Unlock() | |
| 163 | 168 | result.Status = SyncDispatchQueuedAfterTimeout | |
| 164 | 169 | result.Elapsed = time.Since(start) | |
| @@ -172,6 +177,8 @@ func RunSynchronousDispatch(opts SyncDispatchOptions, fn func()) SyncDispatchRes | |||
| 172 | 177 | return result | |
| 173 | 178 | } | |
| 174 | 179 | stateMu.Unlock() | |
| 180 | + | // dispatchStateStarted means the worker already owns fn side effects; wait on | |
| 181 | + | // done so the caller reports completed-after-timeout only after completion. | |
| 175 | 182 | <-done | |
| 176 | 183 | if loadState() == dispatchStateTimedOut { | |
| 177 | 184 | result.Status = SyncDispatchTimedOut | |
| @@ -122,6 +122,9 @@ func RunSynchronousDispatch(opts SyncDispatchOptions, fn func()) SyncDispatchRes | |||||
| 122 | if state != dispatchStateInit { | 122 | if state != dispatchStateInit { | ||
| 123 | return false | 123 | return false | ||
| 124 | } | 124 | } | ||
| 125 | + | // Check deadline and transition to dispatchStateStarted under one lock so | |||
| 126 | + | // the timer cannot mark dispatchStateTimedOut after this callback is allowed | |||
| 127 | + | // to start. Late-start cleanup opts out through AllowLateStartAfterTimeout. | |||
| 125 | if !opts.AllowLateStartAfterTimeout && time.Now().After(deadline) { | 128 | if !opts.AllowLateStartAfterTimeout && time.Now().After(deadline) { | ||
| 126 | state = dispatchStateTimedOut | 129 | state = dispatchStateTimedOut | ||
| 127 | return false | 130 | return false | ||
| @@ -159,6 +162,8 @@ func RunSynchronousDispatch(opts SyncDispatchOptions, fn func()) SyncDispatchRes | |||||
| 159 | stateMu.Lock() | 162 | stateMu.Lock() | ||
| 160 | currentState := state | 163 | currentState := state | ||
| 161 | if opts.AllowLateStartAfterTimeout && currentState == dispatchStateInit { | 164 | if opts.AllowLateStartAfterTimeout && currentState == dispatchStateInit { | ||
| 165 | + | // Cleanup work may remain queued after timer expiry; decision callbacks | |||
| 166 | + | // instead fall through and mark dispatchStateTimedOut while still unstarted. | |||
| 162 | stateMu.Unlock() | 167 | stateMu.Unlock() | ||
| 163 | result.Status = SyncDispatchQueuedAfterTimeout | 168 | result.Status = SyncDispatchQueuedAfterTimeout | ||
| 164 | result.Elapsed = time.Since(start) | 169 | result.Elapsed = time.Since(start) | ||
| @@ -172,6 +177,8 @@ func RunSynchronousDispatch(opts SyncDispatchOptions, fn func()) SyncDispatchRes | |||||
| 172 | return result | 177 | return result | ||
| 173 | } | 178 | } | ||
| 174 | stateMu.Unlock() | 179 | stateMu.Unlock() | ||
| 180 | + | // dispatchStateStarted means the worker already owns fn side effects; wait on | |||
| 181 | + | // done so the caller reports completed-after-timeout only after completion. | |||
| 175 | <-done | 182 | <-done | ||
| 176 | if loadState() == dispatchStateTimedOut { | 183 | if loadState() == dispatchStateTimedOut { | ||
| 177 | result.Status = SyncDispatchTimedOut | 184 | result.Status = SyncDispatchTimedOut | ||