Merge pull request #1499 from jaytaph/gos-281-close-tab

GOS-281: Zone::detach_tab frees a tab's slot without waiting

Author: jaytaph

crates/gosub_engine/src/engine/engine.rs

@@ -1373,6 +1373,37 @@ mod tests {
         engine.shutdown().await.expect("shutdown");
     }
 
+    /// A detached tab gives its `max_tabs` slot back at once, before its worker has
+    /// stopped, so a zone that keeps opening and closing tabs never runs out.
+    #[tokio::test]
+    async fn detached_tab_frees_its_slot() {
+        let mut engine = engine_with_max_zones(1);
+        let _join = tokio::spawn(engine.start().expect("start"));
+        let mut zone = engine.zone_builder().services(services()).create().expect("zone");
+        let max = ZoneConfig::default().max_tabs;
+
+        let mut tabs = Vec::new();
+        for _ in 0..max {
+            tabs.push(zone.tab_builder().create().await.expect("tab"));
+        }
+        assert!(matches!(
+            zone.tab_builder().create().await,
+            Err(EngineError::TabLimitExceeded)
+        ));
+
+        for _ in 0..max * 2 {
+            let closing = zone.detach_tab(tabs.remove(0).tab_id).expect("known tab");
+            assert!(zone.detach_tab(crate::tab::TabId::new()).is_none(), "unknown id");
+            tabs.push(zone.tab_builder().create().await.expect("the slot is free"));
+            tokio::time::timeout(Duration::from_secs(10), closing)
+                .await
+                .expect("the worker stops");
+        }
+
+        engine.close_zone(zone).await;
+        engine.shutdown().await.expect("shutdown");
+    }
+
     /// A page served as an attachment is offered as a download; `RenderDownload` overrides
     /// that and loads the already-spooled body as the page. No second request is made.
     #[tokio::test(flavor = "current_thread")]

crates/gosub_engine/src/engine/zone/zone.rs

@@ -427,23 +427,35 @@ impl<C: RenderConfiguration> Zone<C> {
     /// The worker performs its own teardown on exit (emits `TabClosed`, drops the
     /// tab's session storage). Returns `false` when the tab is unknown.
     pub async fn close_tab(&mut self, tab_id: TabId) -> bool {
-        let Some(info) = self.tabs.remove(&tab_id) else {
-            return false;
-        };
-
-        // A send error means the worker already exited; awaiting the join handle is
-        // still correct in that case.
-        let _ = info.cmd_tx.send(TabCommand::CloseTab).await;
+        match self.detach_tab(tab_id) {
+            Some(closing) => {
+                closing.await;
+                true
+            }
+            None => false,
+        }
+    }
 
+    /// Takes a tab out of the zone, so it no longer counts against `max_tabs`, and
+    /// returns the stopping of it: a future that asks the worker to stop and waits for
+    /// it to exit. For a caller that cannot wait, such as a UI thread, which spawns it.
+    /// Returns `None` when the tab is unknown.
+    pub fn detach_tab(&mut self, tab_id: TabId) -> Option<impl std::future::Future<Output = ()> + Send + 'static> {
+        let info = self.tabs.remove(&tab_id)?;
         let secs = self.context.config_store.get_uint("engine.io_shutdown_secs") as u64;
-        match tokio::time::timeout(std::time::Duration::from_secs(secs), info.join_handle).await {
-            Ok(Ok(())) => {}
-            Ok(Err(e)) => log::warn!("Tab {tab_id} worker join error: {e}"),
-            // The worker keeps running detached and cleans up whenever it exits.
-            Err(_) => log::warn!("Tab {tab_id} worker did not stop within {secs}s"),
-        }
 
-        true
+        Some(async move {
+            // A send error means the worker already exited; awaiting the join handle is
+            // still correct in that case.
+            let _ = info.cmd_tx.send(TabCommand::CloseTab).await;
+
+            match tokio::time::timeout(std::time::Duration::from_secs(secs), info.join_handle).await {
+                Ok(Ok(())) => {}
+                Ok(Err(e)) => log::warn!("Tab {tab_id} worker join error: {e}"),
+                // The worker keeps running detached and cleans up whenever it exits.
+                Err(_) => log::warn!("Tab {tab_id} worker did not stop within {secs}s"),
+            }
+        })
     }
 
     /// Closes every tab in this zone. Used by `GosubEngine::close_zone`.