mirror of
https://github.com/encounter/ghidra-cli.git
synced 2026-07-10 03:18:56 -07:00
fix: resolve Windows CI timeout from bridge lock contention and missing TCP timeouts
- Replace naive 2s sleep in DaemonTestHarness::drop() with proper bridge cleanup: stop_bridge() + poll is_pid_alive() to wait for JVM exit - Add TCP connect timeouts (10s client, 5s bridge checks) to prevent indefinite blocking on partially-alive bridges - Add /T flag to Windows taskkill to kill entire process tree Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 4.6
parent
ee9035fbfe
commit
78c7c83a8e
@@ -134,8 +134,9 @@ pub fn is_bridge_running(project_path: &Path) -> Option<u16> {
|
||||
return None;
|
||||
}
|
||||
|
||||
// Verify TCP connect
|
||||
TcpStream::connect(format!("127.0.0.1:{}", port))
|
||||
// Verify TCP connect (with timeout to avoid long hangs on Windows)
|
||||
let addr: std::net::SocketAddr = format!("127.0.0.1:{}", port).parse().ok()?;
|
||||
TcpStream::connect_timeout(&addr, Duration::from_secs(5))
|
||||
.map(|_| Some(port))
|
||||
.unwrap_or(None)
|
||||
}
|
||||
@@ -151,8 +152,10 @@ pub fn ensure_bridge_running(
|
||||
if let Ok(Some(port)) = read_port_file(project_path) {
|
||||
if let Ok(Some(pid)) = read_pid_file(project_path) {
|
||||
if is_pid_alive(pid) {
|
||||
// Verify TCP connect
|
||||
if TcpStream::connect(format!("127.0.0.1:{}", port)).is_ok() {
|
||||
// Verify TCP connect (with timeout to avoid long hangs on Windows)
|
||||
let addr: std::net::SocketAddr =
|
||||
format!("127.0.0.1:{}", port).parse().unwrap();
|
||||
if TcpStream::connect_timeout(&addr, Duration::from_secs(5)).is_ok() {
|
||||
info!("Bridge already running on port {}", port);
|
||||
return Ok(port);
|
||||
}
|
||||
@@ -346,7 +349,7 @@ pub fn stop_bridge(project_path: &Path) -> Result<()> {
|
||||
#[cfg(windows)]
|
||||
{
|
||||
let _ = std::process::Command::new("taskkill")
|
||||
.args(["/PID", &pid.to_string(), "/F"])
|
||||
.args(["/PID", &pid.to_string(), "/F", "/T"])
|
||||
.output();
|
||||
}
|
||||
}
|
||||
|
||||
+7
-3
@@ -36,9 +36,13 @@ impl BridgeClient {
|
||||
command: &str,
|
||||
args: Option<serde_json::Value>,
|
||||
) -> Result<serde_json::Value> {
|
||||
let mut stream = TcpStream::connect(format!("127.0.0.1:{}", self.port)).map_err(|e| {
|
||||
anyhow::anyhow!("Failed to connect to bridge on port {}: {}", self.port, e)
|
||||
})?;
|
||||
let addr: std::net::SocketAddr = format!("127.0.0.1:{}", self.port)
|
||||
.parse()
|
||||
.map_err(|e| anyhow::anyhow!("Invalid address: {}", e))?;
|
||||
let mut stream =
|
||||
TcpStream::connect_timeout(&addr, Duration::from_secs(10)).map_err(|e| {
|
||||
anyhow::anyhow!("Failed to connect to bridge on port {}: {}", self.port, e)
|
||||
})?;
|
||||
stream.set_read_timeout(Some(Duration::from_secs(300))).ok();
|
||||
stream.set_write_timeout(Some(Duration::from_secs(30))).ok();
|
||||
|
||||
|
||||
@@ -4,3 +4,9 @@
|
||||
|
||||
#[path = "ipc/mod.rs"]
|
||||
pub mod ipc;
|
||||
|
||||
/// Re-export bridge module for integration tests.
|
||||
#[path = "ghidra"]
|
||||
pub mod ghidra {
|
||||
pub mod bridge;
|
||||
}
|
||||
|
||||
+25
-6
@@ -233,14 +233,33 @@ impl DaemonTestHarness {
|
||||
|
||||
impl Drop for DaemonTestHarness {
|
||||
fn drop(&mut self) {
|
||||
// Send shutdown command to bridge
|
||||
let client = ghidra_cli::ipc::client::BridgeClient::new(self.port);
|
||||
let _ = client.shutdown();
|
||||
// Read PID BEFORE shutdown (Java deletes PID file during shutdown)
|
||||
let pid = ghidra_cli::ghidra::bridge::read_pid_file(&self.project_path)
|
||||
.ok()
|
||||
.flatten();
|
||||
|
||||
// Wait for process to exit
|
||||
std::thread::sleep(Duration::from_secs(2));
|
||||
// Use stop_bridge for proper graceful shutdown + force-kill
|
||||
let _ = ghidra_cli::ghidra::bridge::stop_bridge(&self.project_path);
|
||||
|
||||
// Clean up
|
||||
// Wait for process to fully exit and release project lock.
|
||||
// Critical on Windows where JVM cleanup is slow.
|
||||
if let Some(pid) = pid {
|
||||
let max_wait = if cfg!(windows) {
|
||||
Duration::from_secs(30)
|
||||
} else {
|
||||
Duration::from_secs(10)
|
||||
};
|
||||
let start = std::time::Instant::now();
|
||||
while start.elapsed() < max_wait {
|
||||
if !ghidra_cli::ghidra::bridge::is_pid_alive(pid) {
|
||||
break;
|
||||
}
|
||||
std::thread::sleep(Duration::from_millis(500));
|
||||
}
|
||||
}
|
||||
|
||||
// Final cleanup of any remaining stale files
|
||||
let _ = ghidra_cli::ghidra::bridge::cleanup_stale_files(&self.project_path);
|
||||
let _ = std::fs::remove_dir_all(&self.data_dir);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user