From 03fb892ebea66828fd0871db96085072adaaf7bd Mon Sep 17 00:00:00 2001 From: Michael Neale Date: Tue, 2 Dec 2025 09:59:02 +1100 Subject: [PATCH] =?UTF-8?q?fix:=20use=20a=20lock=20to=20ensure=20only=20ne?= =?UTF-8?q?ed=20to=20run=20tunnel=20just=20in=20case=20multiple=20go?= =?UTF-8?q?=E2=80=A6=20(#5885)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- Cargo.lock | 1 + crates/goose-server/Cargo.toml | 1 + crates/goose-server/src/tunnel/mod.rs | 98 ++++++++++++++++++++++++--- 3 files changed, 92 insertions(+), 8 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 6c016d9942..1358745cb1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2871,6 +2871,7 @@ dependencies = [ "chrono", "clap", "config", + "fs2", "futures", "goose", "goose-mcp", diff --git a/crates/goose-server/Cargo.toml b/crates/goose-server/Cargo.toml index b81d53ddfc..cb586eef15 100644 --- a/crates/goose-server/Cargo.toml +++ b/crates/goose-server/Cargo.toml @@ -44,6 +44,7 @@ url = "2.5.7" rand = "0.9.2" hex = "0.4.3" socket2 = "0.6.1" +fs2 = "0.4.3" [target.'cfg(windows)'.dependencies] winreg = { version = "0.55.0" } diff --git a/crates/goose-server/src/tunnel/mod.rs b/crates/goose-server/src/tunnel/mod.rs index d87cc5db9e..67089249a8 100644 --- a/crates/goose-server/src/tunnel/mod.rs +++ b/crates/goose-server/src/tunnel/mod.rs @@ -4,8 +4,11 @@ pub mod lapstone; mod lapstone_test; use crate::configuration::Settings; -use goose::config::Config; +use fs2::FileExt as _; +use goose::config::{paths::Paths, Config}; use serde::{Deserialize, Serialize}; +use std::fs::{File, OpenOptions}; +use std::io::Write; use std::sync::Arc; use tokio::sync::{mpsc, RwLock}; use utoipa::ToSchema; @@ -15,6 +18,53 @@ fn get_server_port() -> anyhow::Result { Ok(settings.port) } +fn get_lock_path() -> std::path::PathBuf { + Paths::config_dir().join("tunnel.lock") +} + +fn try_acquire_tunnel_lock() -> anyhow::Result { + let lock_path = get_lock_path(); + + if let Some(parent) = lock_path.parent() { + std::fs::create_dir_all(parent)?; + } + + let mut file = OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .open(&lock_path)?; + + file.try_lock_exclusive() + .map_err(|_| anyhow::anyhow!("Another goose instance is already running the tunnel"))?; + + writeln!(file, "{}", std::process::id())?; + file.sync_all()?; + + Ok(file) +} + +fn is_tunnel_locked_by_another() -> bool { + let lock_path = get_lock_path(); + + let file = match OpenOptions::new() + .write(true) + .create(true) + .truncate(false) + .open(&lock_path) + { + Ok(f) => f, + Err(_) => return false, + }; + + if file.try_lock_exclusive().is_err() { + return true; + } + + // Lock released when file is dropped + false +} + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq, Default, ToSchema)] #[serde(rename_all = "lowercase")] pub enum TunnelState { @@ -40,6 +90,7 @@ pub struct TunnelManager { lapstone_handle: Arc>>>, restart_tx: Arc>>>, watchdog_handle: Arc>>>, + lock_file: Arc>>, } impl Default for TunnelManager { @@ -56,6 +107,7 @@ impl TunnelManager { lapstone_handle: Arc::new(RwLock::new(None)), restart_tx: Arc::new(RwLock::new(None)), watchdog_handle: Arc::new(RwLock::new(None)), + lock_file: Arc::new(std::sync::Mutex::new(None)), } } @@ -78,13 +130,20 @@ impl TunnelManager { let state = self.state.read().await.clone(); if auto_start && state == TunnelState::Idle { + if is_tunnel_locked_by_another() { + tracing::info!( + "Tunnel already running on another goose instance, skipping auto-start" + ); + return; + } + tracing::info!("Auto-starting tunnel"); match self.start().await { Ok(info) => { tracing::info!("Tunnel auto-started successfully: {:?}", info.url); } Err(e) => { - tracing::error!("Failed to auto-start tunnel: {}", e); + tracing::info!("Tunnel auto-start skipped: {}", e); } } } @@ -117,12 +176,20 @@ impl TunnelManager { tunnel_info.state = state; tunnel_info } - None => TunnelInfo { - state, - url: String::new(), - hostname: String::new(), - secret: String::new(), - }, + None => { + let effective_state = if state == TunnelState::Idle && is_tunnel_locked_by_another() + { + TunnelState::Running + } else { + state + }; + TunnelInfo { + state: effective_state, + url: String::new(), + hostname: String::new(), + secret: String::new(), + } + } } } @@ -182,6 +249,10 @@ impl TunnelManager { if *state != TunnelState::Idle { anyhow::bail!("Tunnel is already running or starting"); } + + let lock = try_acquire_tunnel_lock()?; + *self.lock_file.lock().unwrap() = Some(lock); + *state = TunnelState::Starting; drop(state); @@ -230,6 +301,7 @@ impl TunnelManager { Ok(info) } Err(e) => { + self.release_lock(); *self.state.write().await = TunnelState::Error; Err(e) } @@ -243,6 +315,14 @@ impl TunnelManager { lapstone_handle: self.lapstone_handle.clone(), restart_tx: self.restart_tx.clone(), watchdog_handle: self.watchdog_handle.clone(), + lock_file: self.lock_file.clone(), + } + } + + fn release_lock(&self) { + if let Ok(mut guard) = self.lock_file.lock() { + // Dropping the file releases the lock + guard.take(); } } @@ -255,6 +335,8 @@ impl TunnelManager { lapstone::stop(self.lapstone_handle.clone()).await; + self.release_lock(); + *self.state.write().await = TunnelState::Idle; *self.info.write().await = None;