Skip to content

Commit 635aeae

Browse files
program247365claude
andcommitted
fix(audio): cancel stream downloads at teardown so quit can't freeze
rodio decodes inside the CoreAudio render callback, and StreamDownload::read blocks on a condvar only writer progress or "stream done" can wake. When a live stream goes quiet (e.g. a daily broadcast ending), stream-download's reconnect loop retries forever without signalling, the callback wedges in read, and AudioPlayer teardown then hangs dropping the device sink (AudioOutputUnitStop waits on the callback) — freezing the app on quit until force-quit, which orphans the yt-dlp/ffmpeg children. AudioPlayer now keeps the download's CancellationToken (HTTP and process inputs) and cancels it in a custom Drop before fields drop: stream-download kills the child pipeline and signals stream done, waking any wedged reader. This also reliably reaps yt-dlp/ffmpeg on every track transition and quit. New ignored test reproduces the stall (ffmpeg emits 6s of audio then stalls without EOF) and guards the fix. Companion fixes for deleting the currently-playing history row mid-play: record_playback_time treats the missing row as a no-op instead of erroring (kills the "Record not found" warning smeared over the TUI), and starring always materializes the row first instead of crashing on NotFound. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 6ecbecc commit 635aeae

6 files changed

Lines changed: 141 additions & 14 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,9 @@ rodio = { version = "0.22", default-features = false, features = ["playback", "m
3636
souvlaki = { version = "0.8", default-features = false, features = ["use_zbus"] }
3737
structopt = "0.3.21"
3838
tokio = { version = "1", features = ["rt-multi-thread"] }
39+
# Already in the tree transitively via stream-download; the direct dep is only
40+
# for CancellationToken, used to cancel stream downloads at player teardown.
41+
tokio-util = "0.7"
3942
# Spotify playback. `librespot-playback` has default-features off on purpose:
4043
# it keeps the bundled (older) rodio audio backend out of the tree so there is
4144
# only one rodio. We feed librespot's decoded PCM through our own custom Sink

WORKLOG.md

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -262,3 +262,32 @@
262262
metadata, vortex animation. v0.13.1: [l] Loop added to the footer menu, shown
263263
only where the key is live (playlist track, not solo-looping).
264264
- Both releases: tag → CI arm64 binary → tap formula updated automatically.
265+
266+
## 2026-07-09: Fixed the quit-freeze on stalled live streams (the "deleted the playing row" force-quit)
267+
- Diagnosed the reported freeze: deleting the currently-playing history row was
268+
incidental. Root cause: rodio decodes inside the CoreAudio render callback, and
269+
`StreamDownload::read` blocks on a Condvar that only writer progress or
270+
"stream done" wakes. When a live stream goes quiet (e.g. the daily Claude FM
271+
broadcast ending), stream-download's reconnect loop retries forever without
272+
signalling, the callback wedges in `read`, and `AudioPlayer` teardown then
273+
hangs in `_device` drop (AudioOutputUnitStop waits on the callback) — right
274+
between the two quit warnings, matching the frozen screenshot. Force quit then
275+
orphaned yt-dlp/ffmpeg (found still alive, PPID 1).
276+
- Fix: `AudioPlayer` now keeps the download's `CancellationToken` (HTTP and
277+
process inputs) and cancels it in a custom `Drop` before fields drop.
278+
stream-download's cancelled path kills the child pipeline and signals stream
279+
done, waking any wedged reader. Also fixes child-process cleanup on every
280+
teardown. New ignored test `drop_does_not_hang_on_stalled_process_stream`
281+
(ffmpeg emits 6s of sine then stalls without EOF) reproduced the hang, passes
282+
with the fix.
283+
- Companion fixes for the delete-current-row scenario: `record_playback_time`
284+
treats a missing row as a no-op (respect the delete; kills the "Record not
285+
found" warning smeared over the TUI), and `s`/star always `ensure_track_row`
286+
first so starring after a mid-play delete materializes instead of crashing.
287+
- Verified: cargo test green (80 + 2), stalled-stream test passes in ~9s,
288+
real-world e2e (live lofi stream, quit during playback) exits in ~2s with zero
289+
leftover yt-dlp/ffmpeg. Note: fix is unreleased — needs `make release-patch`
290+
to reach the Homebrew-installed binary Kevin actually runs.
291+
- TUI e2e gotcha for next time: grep the pty log for UI strings fails — ratatui
292+
writes cells with escape codes between characters; use the terminal title
293+
(emitted as one OSC string) as the playback-state signal instead.

src/audio.rs

Lines changed: 70 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ use stream_download::storage::memory::MemoryStorageProvider;
1717
use stream_download::storage::temp::TempStorageProvider;
1818
use stream_download::{Settings, StreamDownload};
1919
use tokio::runtime::Runtime;
20+
use tokio_util::sync::CancellationToken;
2021

2122
use crate::playback_input::PlaybackInput;
2223
use stream_download::http::reqwest::header::{HeaderMap, HeaderName, HeaderValue};
@@ -83,6 +84,11 @@ impl<S: Source> Source for SampleTap<S> {
8384
pub struct AudioPlayer {
8485
// Owns the OS audio output; dropping it stops playback, so keep it alive.
8586
_device: MixerDeviceSink,
87+
// Cancels the stream download at teardown (see `Drop` below). Without it a
88+
// stalled stream (e.g. a live broadcast that went quiet) leaves the audio
89+
// callback blocked inside `StreamDownload::read`, and dropping `_device`
90+
// then waits on that callback forever — freezing the whole app on quit.
91+
download_cancel: Option<CancellationToken>,
8692
_download_runtime: Option<Runtime>,
8793
pub sink: Player,
8894
pub duration: Option<Duration>,
@@ -123,6 +129,7 @@ impl AudioPlayer {
123129
});
124130
return Ok(Self {
125131
_device: device,
132+
download_cancel: None,
126133
_download_runtime: None,
127134
sink,
128135
duration,
@@ -135,7 +142,7 @@ impl AudioPlayer {
135142
});
136143
}
137144

138-
let (reader, duration, runtime, byte_len) = open_input(&input)?;
145+
let (reader, duration, runtime, byte_len, download_cancel) = open_input(&input)?;
139146
let source = decode_input(reader, byte_len)?;
140147

141148
let sample_rate = source.sample_rate().get();
@@ -164,6 +171,7 @@ impl AudioPlayer {
164171

165172
Ok(Self {
166173
_device: device,
174+
download_cancel,
167175
_download_runtime: runtime,
168176
sink,
169177
duration,
@@ -214,7 +222,7 @@ impl AudioPlayer {
214222
if !self.refillable || !self.sink.empty() {
215223
return Ok(false);
216224
}
217-
let (reader, _, _, byte_len) = open_input(&self.input)?;
225+
let (reader, _, _, byte_len, _) = open_input(&self.input)?;
218226
let source = decode_input(reader, byte_len)?;
219227
let tapped = SampleTap {
220228
inner: source,
@@ -225,6 +233,19 @@ impl AudioPlayer {
225233
}
226234
}
227235

236+
/// Runs before the fields drop. Cancelling the download makes stream-download
237+
/// kill the yt-dlp/ffmpeg children and signal "stream done", which wakes any
238+
/// audio callback blocked inside `StreamDownload::read`. Only then can
239+
/// `_device`'s drop (which waits for the callback to return) complete. The
240+
/// runtime processing the cancellation is still alive here — fields drop after.
241+
impl Drop for AudioPlayer {
242+
fn drop(&mut self) {
243+
if let Some(token) = &self.download_cancel {
244+
token.cancel();
245+
}
246+
}
247+
}
248+
228249
fn decode_input(
229250
reader: Box<dyn MediaReader>,
230251
byte_len: Option<u64>,
@@ -238,9 +259,15 @@ fn decode_input(
238259
builder.build().wrap_err("failed to decode audio")
239260
}
240261

241-
fn open_input(
242-
input: &PlaybackInput,
243-
) -> Result<(Box<dyn MediaReader>, Option<Duration>, Option<Runtime>, Option<u64>)> {
262+
type OpenedInput = (
263+
Box<dyn MediaReader>,
264+
Option<Duration>,
265+
Option<Runtime>,
266+
Option<u64>,
267+
Option<CancellationToken>,
268+
);
269+
270+
fn open_input(input: &PlaybackInput) -> Result<OpenedInput> {
244271
match input {
245272
PlaybackInput::File(path) => {
246273
let path_str = path.to_string_lossy();
@@ -252,7 +279,7 @@ fn open_input(
252279

253280
let file = File::open(path).wrap_err("failed to open audio file")?;
254281
let byte_len = file.metadata().ok().map(|m| m.len());
255-
Ok((Box::new(file), duration, None, byte_len))
282+
Ok((Box::new(file), duration, None, byte_len, None))
256283
}
257284
PlaybackInput::HttpStream { url, headers } => {
258285
let runtime = tokio::runtime::Builder::new_multi_thread()
@@ -274,7 +301,8 @@ fn open_input(
274301
.wrap_err("failed to open HTTP audio stream")
275302
})
276303
.wrap_err("failed to open HTTP audio stream")?;
277-
Ok((Box::new(reader), None, Some(runtime), None))
304+
let cancel = reader.cancellation_token();
305+
Ok((Box::new(reader), None, Some(runtime), None, Some(cancel)))
278306
}
279307
PlaybackInput::ProcessStdout {
280308
program,
@@ -302,7 +330,8 @@ fn open_input(
302330
.wrap_err("failed to open process audio stream")
303331
})
304332
.wrap_err("failed to open process audio stream")?;
305-
Ok((Box::new(reader), None, Some(runtime), None))
333+
let cancel = reader.cancellation_token();
334+
Ok((Box::new(reader), None, Some(runtime), None, Some(cancel)))
306335
}
307336
// Spotify is wired up earlier in `AudioPlayer::new` and never reaches
308337
// the file/stream reader path.
@@ -399,6 +428,39 @@ mod tests {
399428
);
400429
}
401430

431+
#[test]
432+
#[ignore = "requires a real audio output device and ffmpeg"]
433+
fn drop_does_not_hang_on_stalled_process_stream() {
434+
use crate::playback_input::{PlaybackInput, ProcessFormat};
435+
use std::time::Duration;
436+
437+
// Emits ~6s of WAV (past the 256 KB stream prefetch), then stalls
438+
// forever without EOF — the shape of a live stream whose upstream
439+
// broadcast went quiet.
440+
let input = PlaybackInput::process_stdout(
441+
"sh",
442+
vec![
443+
"-c".to_string(),
444+
"ffmpeg -v error -f lavfi -i sine=frequency=200:duration=6 -af volume=0.05 -f wav -; sleep 600"
445+
.to_string(),
446+
],
447+
ProcessFormat::Wav,
448+
);
449+
let player = super::AudioPlayer::new(input, false).expect("player should initialize");
450+
// Let playback drain the buffered audio so the decoder is blocked
451+
// inside the audio callback waiting on the stalled stream.
452+
std::thread::sleep(Duration::from_secs(9));
453+
454+
let (done_tx, done_rx) = std::sync::mpsc::channel();
455+
std::thread::spawn(move || {
456+
drop(player);
457+
let _ = done_tx.send(());
458+
});
459+
done_rx
460+
.recv_timeout(Duration::from_secs(10))
461+
.expect("AudioPlayer drop wedged on a stalled stream");
462+
}
463+
402464
#[test]
403465
fn unknown_length_stream_storage_is_bounded() {
404466
let (mut reader, mut writer) = stream_storage_with_buffer(4)

src/play_loop.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -764,11 +764,11 @@ fn dispatch_command(
764764
if let Ok(record) = track_record(track) {
765765
let favorite = {
766766
let storage = storage.lock().unwrap();
767-
// Playlist tracks have no history row of their own;
768-
// starring one materializes it so there's a row to flip.
769-
if state.is_playlist {
770-
storage.ensure_track_row(&record)?;
771-
}
767+
// The current track may have no history row: playlist
768+
// tracks never get one, and the user can delete the row
769+
// mid-play. Starring is a deliberate act — materialize the
770+
// row so there's one to flip instead of erroring out.
771+
storage.ensure_track_row(&record)?;
772772
storage.toggle_favorite(&record.track_key)?
773773
};
774774
state.is_favorite = favorite;

src/storage.rs

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -342,10 +342,16 @@ impl Storage {
342342

343343
let mut connection = establish_connection(&self.db_path)?;
344344
connection.transaction::<_, diesel::result::Error, _>(|conn| {
345+
// A missing row means the user deleted it mid-play; respect the
346+
// delete rather than erroring (or resurrecting the row).
345347
let existing_seconds = tracks::played_tracks
346348
.filter(tracks::track_key.eq(track_key))
347349
.select(tracks::total_play_seconds)
348-
.first::<i64>(conn)?;
350+
.first::<i64>(conn)
351+
.optional()?;
352+
let Some(existing_seconds) = existing_seconds else {
353+
return Ok(());
354+
};
349355

350356
diesel::update(tracks::played_tracks.filter(tracks::track_key.eq(track_key)))
351357
.set(tracks::total_play_seconds.eq(existing_seconds + played_seconds))
@@ -839,6 +845,32 @@ mod tests {
839845
.is_empty());
840846
}
841847

848+
#[test]
849+
fn playback_time_for_deleted_row_is_a_no_op() {
850+
let (_dir, storage) = test_storage();
851+
let record = TrackRecord {
852+
track_key: "yt:YmQ7jRgf4f0".into(),
853+
replay_target: "https://www.youtube.com/watch?v=YmQ7jRgf4f0".into(),
854+
title: "Claude FM 06-11".into(),
855+
platform: "YouTube".into(),
856+
kind: RecordKind::Track,
857+
};
858+
storage.record_play(&record).unwrap();
859+
storage
860+
.delete_by_replay_target(&record.replay_target)
861+
.unwrap();
862+
863+
// Deleting the currently-playing row mid-play must not turn the final
864+
// play-time write into an error, nor resurrect the deleted row.
865+
storage
866+
.record_playback_time(&record.track_key, 42)
867+
.unwrap();
868+
assert!(storage
869+
.list_history(HistorySortField::LastPlayed, false)
870+
.unwrap()
871+
.is_empty());
872+
}
873+
842874
#[test]
843875
fn pull_replica_skips_when_replica_not_newer() {
844876
let dir = tempdir().unwrap();

0 commit comments

Comments
 (0)