From aeaefcdf2443be57829f32f86b4a2a7bab32f687 Mon Sep 17 00:00:00 2001 From: Claude Date: Mon, 26 Oct 2026 07:22:02 +0000 Subject: [PATCH] Fix the app hanging on Connect to Server v0.2.9's Link::start locked the playout to hand it the waveform channel, but the app calls it from the UI thread with that lock already held, so connecting deadlocked the app (the server saw a normal sign-in). The mirror now gets its wave channel from Playout::wave_requests before the swap, and Link::start takes the receiver without locking. The server file picker had the same re-lock while connected; it now reads the library folders from the playout it is given. Adds a test that connects with the playout locked, as the app does. Co-Authored-By: Claude Opus 5.5 (1M context) Claude-Session: https://claude.ai/code/session_0121MtpepbszTWK1oCTggoPk --- src/fileui.rs | 5 ++--- src/link.rs | 16 +++++++++++----- src/server.rs | 49 +++++++++++++++++++++++++++++++++++++++++++++---- src/serverui.rs | 6 ++++-- src/sync.rs | 7 +++++-- 5 files changed, 67 insertions(+), 16 deletions(-) diff --git a/src/fileui.rs b/src/fileui.rs index fca364b..0338087 100644 --- a/src/fileui.rs +++ b/src/fileui.rs @@ -123,7 +123,7 @@ impl App { }); } - pub(super) fn server_picker_window(&mut self, ctx: &egui::Context) { + pub(super) fn server_picker_window(&mut self, ctx: &egui::Context, core: &Playout) { if let Some(p) = self.files.asked.get_mut().take() { self.files.picker = Some(p); } @@ -152,8 +152,7 @@ impl App { } if p.list.is_none() && p.loading.is_none() { let (tx, rx) = mpsc::channel(); - let (fs, dir, all, ctx, folders) = - (fs.clone(), p.dir.clone(), p.all, ctx.clone(), self.core.lock().cfg.library_folders.clone()); + let (fs, dir, all, ctx, folders) = (fs.clone(), p.dir.clone(), p.all, ctx.clone(), core.cfg.library_folders.clone()); thread::spawn(move || { let _ = tx.send(fs.list(&dir, all, &folders)); ctx.request_repaint(); diff --git a/src/link.rs b/src/link.rs index 12d00d1..6cae986 100644 --- a/src/link.rs +++ b/src/link.rs @@ -258,8 +258,16 @@ pub struct Link { impl Link { /// Keep `core` (a mirror made with `Playout::mirror_of`) in step with the server. - /// `cmds` is the receiving end of the mirror's link; `repaint` wakes the UI after each update. - pub fn start(core: Arc>, session: Session, cmds: Receiver, repaint: Box) -> Link { + /// `cmds` is the receiving end of the mirror's link, `waves` the mirror's `wave_requests`; + /// `repaint` wakes the UI after each update. + /// It never locks `core` itself: the app calls it with the playout locked. + pub fn start( + core: Arc>, + session: Session, + cmds: Receiver, + waves: Receiver, + repaint: Box, + ) -> Link { let stop = Arc::new(AtomicBool::new(false)); let token = Arc::new(Mutex::new(session.token.clone())); let new_password = Arc::new(Mutex::new(None)); @@ -285,11 +293,9 @@ impl Link { queued: vec![], }; // the track editor's waveforms come from the server, which has the files - let (wtx, wrx) = crossbeam_channel::unbounded(); - s.core.lock().set_wave_source(wtx); { let (core, base, pin, token, stop) = (s.core.clone(), session.base.clone(), session.pin.clone(), token.clone(), stop.clone()); - thread::spawn(move || fetch_waves(core, base, pin, token, wrx, &stop)); + thread::spawn(move || fetch_waves(core, base, pin, token, waves, &stop)); } let st = stop.clone(); thread::spawn(move || s.run(&st, repaint)); diff --git a/src/server.rs b/src/server.rs index 2bbeb25..dff8c42 100644 --- a/src/server.rs +++ b/src/server.rs @@ -1541,6 +1541,43 @@ mod tests { assert!(sound_after(&on_air, 1.5).is_none(), "silent once the mic is off"); } + /// Connecting the way the app does: the UI thread holds the playout's lock while it swaps in the + /// mirror and starts the link. Starting the link must not lock it again (v0.2.9 hung there). + #[test] + fn connecting_with_the_playout_locked() { + crate::config::set_data_dir(std::env::temp_dir().join(format!("rm-server-test-{}", std::process::id()))); + let station = station_with(vec![row("one"), row("two")]); + let cfg = ServerCfg { + user: "studio".into(), + password_hash: crate::auth::hash("correct horse battery").unwrap(), + bind: "127.0.0.1".into(), + port: 0, + ..Default::default() + }; + let host = + Host { control: Box::new(|_| {}), reapply_audio: Box::new(|| {}), audio_report: Default::default(), forced_silent: true }; + let addr = spawn(station.clone(), cfg, host).unwrap().addr; + let session = crate::link::login(&format!("127.0.0.1:{}", addr.port()), "studio", "correct horse battery", "").unwrap(); + + let app = Arc::new(Mutex::new(Playout::new(Engine::new_detached(), crate::config::Config::default()))); + let (done_tx, done_rx) = crossbeam_channel::bounded(1); + let shared = app.clone(); + thread::spawn(move || { + let mut core = shared.lock(); + let (tx, rx) = crossbeam_channel::unbounded(); + let mut local = Playout::mirror_of(&core, tx); + let waves = local.wave_requests(); + std::mem::swap(&mut *core, &mut local); + let link = crate::link::Link::start(shared.clone(), session, rx, waves, Box::new(|| {})); + drop(core); + let _ = done_tx.send((link, local)); + }); + let (link, _local) = done_rx.recv_timeout(Duration::from_secs(10)).expect("connecting hung with the playout locked"); + wait_for("the first copy", || app.lock().playlist.len() == 2); + assert!(link.state.lock().online); + link.stop(); + } + /// A server and a desktop mirror talking over HTTP: sign-in, both directions of playlist sync, /// settings, and forwarded actions. #[test] @@ -1566,7 +1603,8 @@ mod tests { let local = Playout::new(Engine::new_detached(), crate::config::Config::default()); let (tx, rx) = crossbeam_channel::unbounded(); let mirror = Arc::new(Mutex::new(Playout::mirror_of(&local, tx))); - let link = crate::link::Link::start(mirror.clone(), session, rx, Box::new(|| {})); + let waves = mirror.lock().wave_requests(); + let link = crate::link::Link::start(mirror.clone(), session, rx, waves, Box::new(|| {})); // the server's playlist reaches the app wait_for("the first copy", || mirror.lock().playlist.len() == 2); @@ -1656,7 +1694,8 @@ mod tests { let local = Playout::new(Engine::new_detached(), crate::config::Config::default()); let (tx, rx) = crossbeam_channel::unbounded(); let mirror = Arc::new(Mutex::new(Playout::mirror_of(&local, tx))); - let link = crate::link::Link::start(mirror.clone(), session, rx, Box::new(|| {})); + let waves = mirror.lock().wave_requests(); + let link = crate::link::Link::start(mirror.clone(), session, rx, waves, Box::new(|| {})); wait_for("the first copy", || mirror.lock().playlist.len() == 2); // missing is what the server says, whatever this machine has @@ -1736,7 +1775,8 @@ mod tests { let local = Playout::new(Engine::new_detached(), crate::config::Config::default()); let (tx, rx) = crossbeam_channel::unbounded(); let mirror = Arc::new(Mutex::new(Playout::mirror_of(&local, tx))); - let link = crate::link::Link::start(mirror.clone(), dj.clone(), rx, Box::new(|| {})); + let waves = mirror.lock().wave_requests(); + let link = crate::link::Link::start(mirror.clone(), dj.clone(), rx, waves, Box::new(|| {})); // (other tests may have left playlists in the shared data folder) let n = station.lock().playlist.len(); wait_for("the first copy", || mirror.lock().playlist.len() == n); @@ -1887,7 +1927,8 @@ mod tests { let local = Playout::new(Engine::new_detached(), crate::config::Config::default()); let (tx, rx) = crossbeam_channel::unbounded(); let mirror = Arc::new(Mutex::new(Playout::mirror_of(&local, tx))); - let link = crate::link::Link::start(mirror.clone(), session, rx, Box::new(|| {})); + let waves = mirror.lock().wave_requests(); + let link = crate::link::Link::start(mirror.clone(), session, rx, waves, Box::new(|| {})); wait_for("the playlist over HTTPS", || mirror.lock().playlist.len() == 1); mirror.lock().append_to_tab(0, vec![row("two")]); wait_for("the edit on the server over HTTPS", || station.lock().playlist.len() == 2); diff --git a/src/serverui.rs b/src/serverui.rs index d82c23d..53e1cdc 100644 --- a/src/serverui.rs +++ b/src/serverui.rs @@ -248,7 +248,7 @@ impl App { self.save_session_window(ctx, core); self.server_status_window(ctx, core); self.server_settings_window(ctx); - self.server_picker_window(ctx); + self.server_picker_window(ctx, core); self.server_admin_windows(ctx); } @@ -454,9 +454,11 @@ impl App { core.save_all(); let (tx, rx) = crossbeam_channel::unbounded(); let mut local = Playout::mirror_of(core, tx); + let waves = local.wave_requests(); std::mem::swap(core, &mut local); let ctx = self.ctx.clone(); - let link = Link::start(self.core.clone(), s.clone(), rx, Box::new(move || ctx.request_repaint())); + // (`core` is the locked playout: Link::start must not lock it again, or the app hangs) + let link = Link::start(self.core.clone(), s.clone(), rx, waves, Box::new(move || ctx.request_repaint())); if !s.pin.is_empty() { self.srv.pins.insert(pin_key(&s.base), s.pin.clone()); } diff --git a/src/sync.rs b/src/sync.rs index 26d8113..8caf919 100644 --- a/src/sync.rs +++ b/src/sync.rs @@ -797,10 +797,13 @@ impl Playout { self.waves.get(p).map(|w| WaveWire::of(w, self.cues.get(p).copied())) } - /// From now on this mirror asks the server for waveforms (see `link`). - pub fn set_wave_source(&mut self, tx: crossbeam_channel::Sender) { + /// From now on this mirror asks the server for waveforms: the paths it wants arrive on the + /// returned channel, for `link::Link::start`. + pub fn wave_requests(&mut self) -> crossbeam_channel::Receiver { + let (tx, rx) = crossbeam_channel::unbounded(); self.remote_waves = Some(tx); self.pending.clear(); + rx } /// A waveform the server sent, or why it couldn't.