Fix the app hanging on Connect to Server

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();

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<Mutex<Playout>>, session: Session, cmds: Receiver<Cmd>, repaint: Box<dyn Fn() + Send>) -> 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<Mutex<Playout>>,
+        session: Session,
+        cmds: Receiver<Cmd>,
+        waves: Receiver<std::path::PathBuf>,
+        repaint: Box<dyn Fn() + Send>,
+    ) -> 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));

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);

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());
         }

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<PathBuf>) {
+    /// 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<PathBuf> {
+        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.