Skip to content

Commit 4d5f3df

Browse files
authored
Merge pull request #413 from guerlain-hitier/fix-timer-skip-reset-race
fix: prevent timer from getting stuck after skip or restart
2 parents a89e8b7 + 2a39dfc commit 4d5f3df

2 files changed

Lines changed: 50 additions & 15 deletions

File tree

src-tauri/src/timer/engine.rs

Lines changed: 39 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,9 @@ pub enum TimerCommand {
2323
Skip,
2424
/// Change the total duration; moves engine to Idle so caller must Start.
2525
Reconfigure { duration_secs: u32 },
26+
/// Update the stored duration without altering phase or elapsed time.
27+
/// Used to arm the next round/reset path without clobbering a fresh Start.
28+
Prime { duration_secs: u32 },
2629
/// OS sleep detected: freeze elapsed position, block until WakeResume.
2730
Suspend,
2831
/// OS wake detected: resume from the saved elapsed position.
@@ -123,10 +126,14 @@ fn run_loop(
123126
total_secs = d;
124127
Transition::Stay
125128
}
129+
Ok(TimerCommand::Prime { duration_secs: d }) => {
130+
total_secs = d;
131+
Transition::Stay
132+
}
126133
// Reset while Idle: emit the event so the listener can update
127-
// the frontend and send a follow-up Reconfigure. Without this
128-
// handler the command would be silently swallowed, leaving the
129-
// listener blocked in recv() and the UI stale.
134+
// the frontend and then sync the next-round duration. Without
135+
// this handler the command would be silently swallowed,
136+
// leaving the listener blocked in recv() and the UI stale.
130137
Ok(TimerCommand::Reset) => {
131138
elapsed_secs = 0;
132139
let _ = event_tx.send(TimerEvent::Reset);
@@ -166,6 +173,10 @@ fn run_loop(
166173
elapsed_secs = 0;
167174
Transition::To(Phase::Idle)
168175
}
176+
Ok(TimerCommand::Prime { duration_secs: d }) => {
177+
total_secs = d;
178+
Transition::Stay
179+
}
169180
Ok(TimerCommand::Shutdown) | Err(_) => Transition::Break,
170181
_ => Transition::Stay,
171182
},
@@ -218,6 +229,10 @@ fn run_loop(
218229
elapsed_secs = 0;
219230
Transition::To(Phase::Idle)
220231
}
232+
Ok(TimerCommand::Prime { duration_secs: d }) => {
233+
total_secs = d;
234+
Transition::Stay
235+
}
221236
Ok(TimerCommand::Shutdown) => Transition::Break,
222237
_ => Transition::Stay,
223238
}
@@ -488,6 +503,27 @@ mod tests {
488503
);
489504
}
490505

506+
#[test]
507+
fn prime_updates_duration_without_cancelling_fresh_start() {
508+
// Models the skip/reset race: the UI sends Start for the next round
509+
// before the listener's follow-up duration update reaches the engine.
510+
let (handle, rx) = spawn(10, TICK);
511+
handle.send(TimerCommand::Start);
512+
std::thread::sleep(TICK / 2);
513+
handle.send(TimerCommand::Prime { duration_secs: 3 });
514+
515+
let events = collect_until_complete(&rx, Duration::from_secs(2));
516+
let ticks = events
517+
.iter()
518+
.filter(|e| matches!(e, TimerEvent::Tick { .. }))
519+
.count();
520+
assert_eq!(ticks, 3, "Prime to 3s should keep the timer running and yield 3 ticks, got {ticks}");
521+
assert!(
522+
matches!(events.last(), Some(TimerEvent::Complete { .. })),
523+
"last event must be Complete after priming a fresh start"
524+
);
525+
}
526+
491527
#[test]
492528
fn drift_complete_within_tolerance() {
493529
// 5 ticks at TICK (20 ms) = nominal 100 ms.

src-tauri/src/timer/mod.rs

Lines changed: 11 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -139,9 +139,8 @@ impl TimerController {
139139
log::info!("[timer] reset");
140140
self.sequence.lock().unwrap().reset();
141141
// Send only Reset — the event listener's Reset handler will follow up
142-
// with Reconfigure once the engine is confirmed Idle. Sending
143-
// Reconfigure here first would push the engine into Idle before Reset
144-
// arrives, causing Reset to be silently dropped and the UI to freeze.
142+
// with Prime once the engine is confirmed Idle. Sending a duration
143+
// update here first would race the Reset and can leave the UI stale.
145144
self.engine.send(TimerCommand::Reset);
146145
}
147146

@@ -330,8 +329,9 @@ fn listen_events(
330329
s.is_running = false;
331330
}
332331

333-
// Prepare the engine for the next round.
334-
engine.send(TimerCommand::Reconfigure {
332+
// Arm the next round's duration without risking a late
333+
// reconfigure that kicks a freshly-started timer back to Idle.
334+
engine.send(TimerCommand::Prime {
335335
duration_secs: next_duration,
336336
});
337337

@@ -455,18 +455,17 @@ fn listen_events(
455455
websocket::broadcast_reset(&ws);
456456
}
457457

458-
// Reconfigure the engine with the current round's duration so
459-
// the next Start uses the correct (possibly settings-updated)
460-
// total. This is safe here because the engine is guaranteed
461-
// to be in Idle when it emits Reset (all three phases —
462-
// Running, Paused, and now Idle — transition to/stay Idle
463-
// before sending the event).
458+
// Prime the engine with the current round's duration so the
459+
// next Start uses the correct (possibly settings-updated)
460+
// total. Using the lighter-weight command here avoids a race
461+
// where a fast user click on Start is immediately clobbered by
462+
// a late follow-up duration update.
464463
let duration = {
465464
let seq = sequence.lock().unwrap();
466465
let s = settings.lock().unwrap();
467466
seq.current_duration_secs(&s)
468467
};
469-
engine.send(TimerCommand::Reconfigure { duration_secs: duration });
468+
engine.send(TimerCommand::Prime { duration_secs: duration });
470469

471470
// Reset tray to idle (empty arc).
472471
let rt = sequence.lock().unwrap().current_round.as_str().to_string();

0 commit comments

Comments
 (0)