Jake
d7363cc913
The accumulator task was fire-and-forget — `tokio::spawn` without
retaining the JoinHandle. `stop_native_capture` sent a stop signal,
slept 50ms, and returned; the worker could still be running its
final flush and appending to `all_samples` when the next
`start_native_capture` cleared it. Rapid start→stop→start could
leak tail samples from one session into another.
Replace `NativeCaptureState.stop_tx` with `worker:
AsyncMutex<Option<CaptureWorker>>`, where CaptureWorker owns both
the stop sender and the spawned task's JoinHandle. New helper
`stop_worker(worker)` sends stop, drops the sender, and `.await`s
the join. Both the prior-worker tear-down in `start_native_capture`
and `stop_native_capture` itself go through the helper, so the
worker is always fully terminated before any downstream read or
next-session cleanup.
AsyncMutex (not std::sync::Mutex) because the stop path awaits
while holding the lock. Also drops the 50ms sleep from
stop_native_capture — the join is an exact barrier.
Two regression tests:
- stop_worker_awaits_full_termination_no_writes_after_join:
synthetic worker with an atomic counter and a flush marker.
After stop_worker the flush must have run and no further
writes may appear.
- stop_worker_is_idempotent_on_a_worker_that_has_already_exited:
tasks that stop themselves must still join cleanly.
A full cpal-backed start→stop→start integration test is not
feasible in Linux CI without an audio device. The component tests
cover the invariant the real flow depends on.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
3.0 KiB
RB-06 MAJOR: native capture worker is detached, can outlive stop/start
Severity: MAJOR
Path: src-tauri/src/commands/audio.rs:46-228
Source: 2026-04-22 code review
Labels: release-blocker, major, concurrency, audio
Status: RESOLVED (2026-04-22)
Resolution
Introduced CaptureWorker { stop_tx, join: JoinHandle<()> } as the
single handle type retained in state. NativeCaptureState.stop_tx
(a std::sync::Mutex<Option<Sender>>) became worker: tokio::sync::Mutex<Option<CaptureWorker>> — the async mutex is
required because the stop path awaits the join while holding the
lock, and holding a blocking mutex across an await is a bug pattern
we don't want to ship.
New helper stop_worker(worker) sends the stop signal, drops the
sender, then join.awaits the task. Errors from join (panic /
cancellation) are logged and swallowed; the caller needs the
synchronisation barrier, not the task's return value.
Both lifecycle paths route through the helper:
start_native_capture— before opening a new capture, if a previous worker is resident, stop it and await termination. This removes the race where the old worker's final flush could append toall_samplesafter the new path cleared it.stop_native_capture— take the worker, stop_worker, then readall_samples. The previous 50ms sleep is no longer needed — the join barrier is exact.
Two regression tests in commands::audio::tests:
stop_worker_awaits_full_termination_no_writes_after_join— synthetic worker bumps an atomic counter in a loop, applies a flush marker at exit. Post-stop-worker the flush marker must be set and no further writes must appear on a subsequent sleep.stop_worker_is_idempotent_on_a_worker_that_has_already_exited— a task that finished on its own must still be join-able without hang or panic.
The full cpal-backed start→stop→start integration test the original issue asks for is not feasible in a Linux CI without an audio device. The component test above covers the underlying invariant the real workflow depends on.
Problem
start_native_capture and stop_native_capture coordinate through a channel but never retain the spawned worker handle. A previous capture can still be flushing / appending after stop_native_capture clears all_samples and before a new start_native_capture takes it — output can be truncated or contaminated with cross-session samples.
Acceptance
- Store the worker's
JoinHandlein the native capture state. stop_native_captureawaits the handle before returning — start/stop/start is fully serialised.- Regression test: rapid start → stop → start sequence produces two distinct samples vectors with no cross-session leakage.
Fix scope
Medium. Requires adding JoinHandle storage and making the stop path await cleanly — probably needs a small refactor of the native capture state struct.
Dependencies
- Independent of other items, though the fix pattern (retain handles, join on stop) mirrors what RB-04 will do for the live-session worker.