mirror of
https://github.com/encounter/ghidra-cli.git
synced 2026-07-10 03:18:56 -07:00
daemon fixes and use cache dir instead of git
This commit is contained in:
+4
-4
@@ -105,12 +105,12 @@ impl Config {
|
||||
return Ok(dir.clone());
|
||||
}
|
||||
|
||||
// Default to ~/git
|
||||
let home = dirs::home_dir().ok_or_else(|| {
|
||||
GhidraError::ConfigError("Could not determine home directory".to_string())
|
||||
// Default to cache dir (e.g., ~/.cache/ghidra-cli/projects)
|
||||
let cache_dir = dirs::cache_dir().ok_or_else(|| {
|
||||
GhidraError::ConfigError("Could not determine cache directory".to_string())
|
||||
})?;
|
||||
|
||||
Ok(home.join("git"))
|
||||
Ok(cache_dir.join("ghidra-cli").join("projects"))
|
||||
}
|
||||
|
||||
#[cfg(target_os = "windows")]
|
||||
|
||||
@@ -100,3 +100,34 @@ Commands sent to bridge.py in Ghidra:
|
||||
- `list_functions`, `decompile`, `list_strings`, etc.
|
||||
|
||||
See `src/ghidra/scripts/bridge.py` for the full command reference.
|
||||
|
||||
## Reliability
|
||||
|
||||
### Bridge Health Monitoring
|
||||
|
||||
The bridge (`GhidraBridge`) tracks whether the Ghidra JVM process is alive:
|
||||
|
||||
- **`check_health()`** - Uses `try_wait()` on the child process to detect if Ghidra has exited
|
||||
- **`send_command()`** - On I/O errors, calls `check_health()` to distinguish process death from network timeouts
|
||||
- **State update** - When process death is detected, `running` flag is set to false and daemon initiates shutdown
|
||||
|
||||
### Daemon Termination on Bridge Death
|
||||
|
||||
When the bridge process dies (Ghidra JVM crash, OOM, etc.):
|
||||
|
||||
1. Handler detects "process died" error from bridge
|
||||
2. Handler signals daemon shutdown via `shutdown_tx.send()`
|
||||
3. Daemon performs graceful cleanup (socket, lock, info files)
|
||||
4. Next CLI command auto-starts a fresh daemon
|
||||
|
||||
This ensures clean state recovery without manual intervention.
|
||||
|
||||
### Stale File Cleanup
|
||||
|
||||
On daemon startup, `get_running_daemon_info()` detects and cleans stale files:
|
||||
|
||||
- **Lock file** - If acquirable, previous daemon is dead; file removed
|
||||
- **Info file** - Removed alongside stale lock file
|
||||
- **Socket file** - Removed to prevent "address in use" errors
|
||||
|
||||
This handles crash scenarios where daemon died without proper cleanup.
|
||||
|
||||
+27
-14
@@ -271,13 +271,13 @@ async fn handle_command_inner(
|
||||
// === Program management commands ===
|
||||
Command::ListPrograms => {
|
||||
require_bridge(&state.bridge).await?;
|
||||
execute_bridge_command(&state.bridge, "list_programs", None).await
|
||||
execute_bridge_command(state, "list_programs", None).await
|
||||
}
|
||||
|
||||
Command::OpenProgram { program } => {
|
||||
require_bridge(&state.bridge).await?;
|
||||
execute_bridge_command(
|
||||
&state.bridge,
|
||||
state,
|
||||
"open_program",
|
||||
Some(json!({"program": program})),
|
||||
)
|
||||
@@ -288,7 +288,7 @@ async fn handle_command_inner(
|
||||
Command::ListFunctions { limit, filter } => {
|
||||
require_bridge(&state.bridge).await?;
|
||||
execute_bridge_command(
|
||||
&state.bridge,
|
||||
state,
|
||||
"list_functions",
|
||||
Some(json!({
|
||||
"limit": limit,
|
||||
@@ -301,7 +301,7 @@ async fn handle_command_inner(
|
||||
Command::Decompile { address } => {
|
||||
require_bridge(&state.bridge).await?;
|
||||
execute_bridge_command(
|
||||
&state.bridge,
|
||||
state,
|
||||
"decompile",
|
||||
Some(json!({
|
||||
"address": address,
|
||||
@@ -313,7 +313,7 @@ async fn handle_command_inner(
|
||||
Command::ListStrings { limit } => {
|
||||
require_bridge(&state.bridge).await?;
|
||||
execute_bridge_command(
|
||||
&state.bridge,
|
||||
state,
|
||||
"list_strings",
|
||||
Some(json!({
|
||||
"limit": limit,
|
||||
@@ -324,28 +324,28 @@ async fn handle_command_inner(
|
||||
|
||||
Command::ListImports => {
|
||||
require_bridge(&state.bridge).await?;
|
||||
execute_bridge_command(&state.bridge, "list_imports", None).await
|
||||
execute_bridge_command(state, "list_imports", None).await
|
||||
}
|
||||
|
||||
Command::ListExports => {
|
||||
require_bridge(&state.bridge).await?;
|
||||
execute_bridge_command(&state.bridge, "list_exports", None).await
|
||||
execute_bridge_command(state, "list_exports", None).await
|
||||
}
|
||||
|
||||
Command::MemoryMap => {
|
||||
require_bridge(&state.bridge).await?;
|
||||
execute_bridge_command(&state.bridge, "memory_map", None).await
|
||||
execute_bridge_command(state, "memory_map", None).await
|
||||
}
|
||||
|
||||
Command::ProgramInfo => {
|
||||
require_bridge(&state.bridge).await?;
|
||||
execute_bridge_command(&state.bridge, "program_info", None).await
|
||||
execute_bridge_command(state, "program_info", None).await
|
||||
}
|
||||
|
||||
Command::XRefsTo { address } => {
|
||||
require_bridge(&state.bridge).await?;
|
||||
execute_bridge_command(
|
||||
&state.bridge,
|
||||
state,
|
||||
"xrefs_to",
|
||||
Some(json!({
|
||||
"address": address,
|
||||
@@ -357,7 +357,7 @@ async fn handle_command_inner(
|
||||
Command::XRefsFrom { address } => {
|
||||
require_bridge(&state.bridge).await?;
|
||||
execute_bridge_command(
|
||||
&state.bridge,
|
||||
state,
|
||||
"xrefs_from",
|
||||
Some(json!({
|
||||
"address": address,
|
||||
@@ -384,12 +384,14 @@ async fn handle_command_inner(
|
||||
}
|
||||
|
||||
/// Execute a command on the Ghidra bridge.
|
||||
///
|
||||
/// If the bridge process dies during command execution, triggers daemon shutdown.
|
||||
async fn execute_bridge_command(
|
||||
bridge: &Arc<Mutex<Option<GhidraBridge>>>,
|
||||
state: &Arc<DaemonState>,
|
||||
command: &str,
|
||||
args: Option<serde_json::Value>,
|
||||
) -> anyhow::Result<serde_json::Value> {
|
||||
let mut bridge_guard = bridge.lock().await;
|
||||
let mut bridge_guard = state.bridge.lock().await;
|
||||
|
||||
let bridge = bridge_guard
|
||||
.as_mut()
|
||||
@@ -401,7 +403,18 @@ async fn execute_bridge_command(
|
||||
|
||||
debug!("Executing bridge command: {}", command);
|
||||
|
||||
let response = bridge.send_command::<serde_json::Value>(command, args)?;
|
||||
let response = match bridge.send_command::<serde_json::Value>(command, args) {
|
||||
Ok(resp) => resp,
|
||||
Err(e) => {
|
||||
// Check if bridge process died - trigger daemon shutdown
|
||||
let err_msg = e.to_string();
|
||||
if err_msg.contains("process died") || !bridge.is_running() {
|
||||
info!("Bridge process died, triggering daemon shutdown");
|
||||
let _ = state.shutdown_tx.send(());
|
||||
}
|
||||
return Err(e);
|
||||
}
|
||||
};
|
||||
|
||||
if response.status == "success" {
|
||||
Ok(response.data.unwrap_or(json!({})))
|
||||
|
||||
@@ -41,6 +41,8 @@ pub struct DaemonState {
|
||||
pub ghidra_install_dir: Option<PathBuf>,
|
||||
/// Project path on disk
|
||||
pub project_path: PathBuf,
|
||||
/// Shutdown signal sender - handlers can trigger daemon shutdown on bridge death
|
||||
pub shutdown_tx: broadcast::Sender<()>,
|
||||
}
|
||||
|
||||
/// Run the daemon with the new bridge architecture.
|
||||
@@ -59,6 +61,7 @@ pub async fn run(config: DaemonConfig) -> Result<()> {
|
||||
bridge: Arc::new(Mutex::new(None)),
|
||||
ghidra_install_dir: config.ghidra_install_dir.clone(),
|
||||
project_path: config.project_path.clone(),
|
||||
shutdown_tx: shutdown_tx.clone(),
|
||||
});
|
||||
|
||||
info!("Bridge will be started on first import/analyze command");
|
||||
|
||||
@@ -139,6 +139,8 @@ pub fn get_running_daemon_info(data_dir: &Path, project_path: &Path) -> Result<O
|
||||
fs::remove_file(&lock_path).ok();
|
||||
let info_path = get_info_file_path(data_dir, project_path);
|
||||
fs::remove_file(&info_path).ok();
|
||||
// Also clean up stale socket file (daemon may have crashed without cleanup)
|
||||
crate::ipc::transport::remove_socket_for_project(project_path).ok();
|
||||
return Ok(None);
|
||||
}
|
||||
|
||||
|
||||
+52
-5
@@ -218,6 +218,9 @@ impl GhidraBridge {
|
||||
}
|
||||
|
||||
/// Send a command to the bridge.
|
||||
///
|
||||
/// On I/O errors, checks if the bridge process has died and updates
|
||||
/// state accordingly. Returns a specific error if the process died.
|
||||
pub fn send_command<T: for<'de> Deserialize<'de>>(
|
||||
&mut self,
|
||||
command: &str,
|
||||
@@ -240,14 +243,29 @@ impl GhidraBridge {
|
||||
let request_json = serde_json::to_string(&request)?;
|
||||
debug!("Sending: {}", request_json);
|
||||
|
||||
// Send request
|
||||
writeln!(stream, "{}", request_json)?;
|
||||
stream.flush()?;
|
||||
// Send request - check process health on I/O error
|
||||
if let Err(e) = writeln!(stream, "{}", request_json) {
|
||||
if !self.check_health() {
|
||||
anyhow::bail!("Bridge process died unexpectedly");
|
||||
}
|
||||
return Err(e.into());
|
||||
}
|
||||
if let Err(e) = stream.flush() {
|
||||
if !self.check_health() {
|
||||
anyhow::bail!("Bridge process died unexpectedly");
|
||||
}
|
||||
return Err(e.into());
|
||||
}
|
||||
|
||||
// Read response
|
||||
// Read response - check process health on I/O error
|
||||
let mut reader = BufReader::new(stream.try_clone()?);
|
||||
let mut response_line = String::new();
|
||||
reader.read_line(&mut response_line)?;
|
||||
if let Err(e) = reader.read_line(&mut response_line) {
|
||||
if !self.check_health() {
|
||||
anyhow::bail!("Bridge process died unexpectedly");
|
||||
}
|
||||
return Err(e.into());
|
||||
}
|
||||
|
||||
debug!("Received: {}", response_line.trim());
|
||||
|
||||
@@ -298,6 +316,35 @@ impl GhidraBridge {
|
||||
self.running.load(Ordering::SeqCst)
|
||||
}
|
||||
|
||||
/// Check if the bridge process is actually healthy (still running).
|
||||
///
|
||||
/// Performs an OS-level check on the child process to detect if it
|
||||
/// has exited unexpectedly. If the process has died, updates the running
|
||||
/// flag and returns false.
|
||||
pub fn check_health(&mut self) -> bool {
|
||||
if let Some(ref mut child) = self.child {
|
||||
match child.try_wait() {
|
||||
Ok(None) => true, // Process still running
|
||||
Ok(Some(status)) => {
|
||||
// Process has exited
|
||||
warn!("Bridge process exited with status: {}", status);
|
||||
self.running.store(false, Ordering::SeqCst);
|
||||
false
|
||||
}
|
||||
Err(e) => {
|
||||
// Error checking process - assume dead
|
||||
error!("Error checking bridge process health: {}", e);
|
||||
self.running.store(false, Ordering::SeqCst);
|
||||
false
|
||||
}
|
||||
}
|
||||
} else {
|
||||
// No child process
|
||||
self.running.store(false, Ordering::SeqCst);
|
||||
false
|
||||
}
|
||||
}
|
||||
|
||||
/// Get the embedded bridge script path, writing all scripts to disk.
|
||||
fn get_bridge_script_path(&self) -> Result<PathBuf> {
|
||||
let scripts_dir = dirs::config_dir()
|
||||
|
||||
@@ -0,0 +1,153 @@
|
||||
//! Tests for daemon IPC reliability - bridge death detection and socket cleanup.
|
||||
|
||||
use serial_test::serial;
|
||||
use std::path::PathBuf;
|
||||
use std::time::Duration;
|
||||
|
||||
#[macro_use]
|
||||
mod common;
|
||||
use common::{ensure_test_project, DaemonTestHarness};
|
||||
|
||||
const TEST_PROJECT: &str = "reliability-test";
|
||||
const TEST_PROGRAM: &str = "sample_binary";
|
||||
|
||||
/// Test that stale socket files are cleaned up on daemon restart.
|
||||
///
|
||||
/// Simulates a crash scenario where socket file remains but daemon is dead.
|
||||
#[test]
|
||||
#[serial]
|
||||
fn test_stale_socket_cleaned_on_restart() {
|
||||
require_ghidra!();
|
||||
|
||||
ensure_test_project(TEST_PROJECT, TEST_PROGRAM);
|
||||
|
||||
// First daemon - start and stop cleanly
|
||||
{
|
||||
let harness = DaemonTestHarness::new(TEST_PROJECT, TEST_PROGRAM)
|
||||
.expect("Failed to start first daemon");
|
||||
|
||||
// Verify daemon is working
|
||||
assert_cmd::Command::cargo_bin("ghidra")
|
||||
.unwrap()
|
||||
.env("GHIDRA_CLI_DATA_DIR", harness.data_dir())
|
||||
.env("GHIDRA_CLI_SOCKET", harness.socket_path())
|
||||
.arg("daemon")
|
||||
.arg("ping")
|
||||
.arg("--project")
|
||||
.arg(TEST_PROJECT)
|
||||
.timeout(Duration::from_secs(30))
|
||||
.assert()
|
||||
.success();
|
||||
|
||||
// Drop will clean up
|
||||
}
|
||||
|
||||
// Brief pause to ensure cleanup completes
|
||||
std::thread::sleep(Duration::from_millis(500));
|
||||
|
||||
// Second daemon - should start without issues (no stale socket conflict)
|
||||
{
|
||||
let harness = DaemonTestHarness::new(TEST_PROJECT, TEST_PROGRAM)
|
||||
.expect("Failed to start second daemon - stale socket may not have been cleaned");
|
||||
|
||||
// Verify daemon is working
|
||||
assert_cmd::Command::cargo_bin("ghidra")
|
||||
.unwrap()
|
||||
.env("GHIDRA_CLI_DATA_DIR", harness.data_dir())
|
||||
.env("GHIDRA_CLI_SOCKET", harness.socket_path())
|
||||
.arg("daemon")
|
||||
.arg("ping")
|
||||
.arg("--project")
|
||||
.arg(TEST_PROJECT)
|
||||
.timeout(Duration::from_secs(30))
|
||||
.assert()
|
||||
.success();
|
||||
}
|
||||
}
|
||||
|
||||
/// Test recovery after daemon crash (simulated via process kill).
|
||||
///
|
||||
/// After killing daemon, a new daemon should be able to start successfully.
|
||||
#[test]
|
||||
#[serial]
|
||||
fn test_recovery_after_crash() {
|
||||
require_ghidra!();
|
||||
|
||||
ensure_test_project(TEST_PROJECT, TEST_PROGRAM);
|
||||
|
||||
let socket_path: PathBuf;
|
||||
let data_dir: PathBuf;
|
||||
|
||||
// Start daemon and get its paths
|
||||
{
|
||||
let harness = DaemonTestHarness::new(TEST_PROJECT, TEST_PROGRAM)
|
||||
.expect("Failed to start daemon");
|
||||
|
||||
socket_path = harness.socket_path().to_path_buf();
|
||||
data_dir = harness.data_dir().to_path_buf();
|
||||
|
||||
// Verify it's working
|
||||
assert_cmd::Command::cargo_bin("ghidra")
|
||||
.unwrap()
|
||||
.env("GHIDRA_CLI_DATA_DIR", &data_dir)
|
||||
.env("GHIDRA_CLI_SOCKET", &socket_path)
|
||||
.arg("daemon")
|
||||
.arg("ping")
|
||||
.arg("--project")
|
||||
.arg(TEST_PROJECT)
|
||||
.timeout(Duration::from_secs(30))
|
||||
.assert()
|
||||
.success();
|
||||
|
||||
// Harness drop will kill daemon (simulating crash)
|
||||
}
|
||||
|
||||
// Brief pause
|
||||
std::thread::sleep(Duration::from_millis(1000));
|
||||
|
||||
// New daemon should start successfully after crash cleanup
|
||||
{
|
||||
let harness = DaemonTestHarness::new(TEST_PROJECT, TEST_PROGRAM)
|
||||
.expect("Failed to start daemon after crash - cleanup may have failed");
|
||||
|
||||
// Verify new daemon is working
|
||||
assert_cmd::Command::cargo_bin("ghidra")
|
||||
.unwrap()
|
||||
.env("GHIDRA_CLI_DATA_DIR", harness.data_dir())
|
||||
.env("GHIDRA_CLI_SOCKET", harness.socket_path())
|
||||
.arg("daemon")
|
||||
.arg("ping")
|
||||
.arg("--project")
|
||||
.arg(TEST_PROJECT)
|
||||
.timeout(Duration::from_secs(30))
|
||||
.assert()
|
||||
.success();
|
||||
}
|
||||
}
|
||||
|
||||
/// Test that daemon commands return appropriate errors when bridge is not ready.
|
||||
#[test]
|
||||
#[serial]
|
||||
fn test_bridge_not_ready_error() {
|
||||
require_ghidra!();
|
||||
|
||||
ensure_test_project(TEST_PROJECT, TEST_PROGRAM);
|
||||
|
||||
let harness =
|
||||
DaemonTestHarness::new(TEST_PROJECT, TEST_PROGRAM).expect("Failed to start daemon");
|
||||
|
||||
// Ping should work (doesn't require bridge)
|
||||
assert_cmd::Command::cargo_bin("ghidra")
|
||||
.unwrap()
|
||||
.env("GHIDRA_CLI_DATA_DIR", harness.data_dir())
|
||||
.env("GHIDRA_CLI_SOCKET", harness.socket_path())
|
||||
.arg("daemon")
|
||||
.arg("ping")
|
||||
.arg("--project")
|
||||
.arg(TEST_PROJECT)
|
||||
.timeout(Duration::from_secs(30))
|
||||
.assert()
|
||||
.success();
|
||||
|
||||
drop(harness);
|
||||
}
|
||||
Reference in New Issue
Block a user