Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 31 additions & 3 deletions src/cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -307,8 +307,7 @@ impl<'a> CmdLineRunner<'a> {
return self.execute_raw();
}
let mut cp = self
.cmd
.spawn()
.spawn_with_etxtbsy_retry()
.wrap_err_with(|| format!("failed to execute command: {self}"))?;
let id = cp.id();
RUNNING_PIDS.lock().unwrap().insert(id);
Expand Down Expand Up @@ -411,13 +410,42 @@ impl<'a> CmdLineRunner<'a> {
}

fn execute_raw(mut self) -> Result<()> {
let status = self.cmd.spawn()?.wait()?;
let status = self.spawn_with_etxtbsy_retry()?.wait()?;
match status.success() {
true => Ok(()),
false => self.on_error(String::new(), status),
}
}

/// Retry spawning a process if it fails with ETXTBSY (Text file busy).
/// This can happen on Linux when executing a binary that was just written/extracted,
/// as the file descriptor may not be fully closed yet.
fn spawn_with_etxtbsy_retry(&mut self) -> std::io::Result<std::process::Child> {
let mut attempt = 0;
loop {
match self.cmd.spawn() {
Ok(child) => return Ok(child),
Err(err) if Self::is_etxtbsy(&err) && attempt < 3 => {
attempt += 1;
trace!("retrying spawn after ETXTBSY (attempt {}/3)", attempt);
Comment on lines +424 to +430

Copilot AI Feb 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The magic number 3 for maximum retry attempts should be extracted as a named constant (e.g., const MAX_ETXTBSY_RETRIES: u32 = 3;) to improve maintainability and make the retry policy more explicit.

Suggested change
let mut attempt = 0;
loop {
match self.cmd.spawn() {
Ok(child) => return Ok(child),
Err(err) if Self::is_etxtbsy(&err) && attempt < 3 => {
attempt += 1;
trace!("retrying spawn after ETXTBSY (attempt {}/3)", attempt);
const MAX_ETXTBSY_RETRIES: i32 = 3;
let mut attempt = 0;
loop {
match self.cmd.spawn() {
Ok(child) => return Ok(child),
Err(err) if Self::is_etxtbsy(&err) && attempt < MAX_ETXTBSY_RETRIES => {
attempt += 1;
trace!(
"retrying spawn after ETXTBSY (attempt {}/{})",
attempt,
MAX_ETXTBSY_RETRIES
);

Copilot uses AI. Check for mistakes.
// Exponential backoff: 50ms, 100ms, 200ms
std::thread::sleep(std::time::Duration::from_millis(50 * (1 << (attempt - 1))));

Copilot AI Feb 2, 2026

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The magic number 50 for the base backoff delay should be extracted as a named constant (e.g., const ETXTBSY_BASE_DELAY_MS: u64 = 50;) to improve maintainability and make the backoff timing more visible.

Copilot uses AI. Check for mistakes.
}
Err(err) => return Err(err),
}
}
}
Comment on lines +423 to +437

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The retry logic uses magic numbers for the maximum number of retries (3) and the initial backoff delay (50ms). It's a good practice to define these as named constants to improve readability and make them easier to modify in the future. I'd suggest defining them as const within the function since they are not used elsewhere.

    fn spawn_with_etxtbsy_retry(&mut self) -> std::io::Result<std::process::Child> {
        const MAX_RETRIES: u32 = 3;
        const INITIAL_BACKOFF_MS: u64 = 50;

        let mut attempt = 0;
        loop {
            match self.cmd.spawn() {
                Ok(child) => return Ok(child),
                Err(err) if Self::is_etxtbsy(&err) && attempt < MAX_RETRIES => {
                    attempt += 1;
                    trace!(
                        "retrying spawn after ETXTBSY (attempt {}/{})",
                        attempt,
                        MAX_RETRIES
                    );
                    // Exponential backoff: 50ms, 100ms, 200ms
                    let backoff_ms = INITIAL_BACKOFF_MS * (1 << (attempt - 1));
                    std::thread::sleep(std::time::Duration::from_millis(backoff_ms));
                }
                Err(err) => return Err(err),
            }
        }
    }


#[cfg(unix)]
fn is_etxtbsy(err: &std::io::Error) -> bool {
err.raw_os_error() == Some(nix::errno::Errno::ETXTBSY as i32)
}

#[cfg(not(unix))]
fn is_etxtbsy(_err: &std::io::Error) -> bool {
false
}

fn on_stdout(&self, line: String) {
let _lock = OUTPUT_LOCK.lock().unwrap();
if let Some(on_stdout) = &self.on_stdout {
Expand Down
Loading