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.