Fix ETXTBSY on Linux during update (#8468)#8469
Fix ETXTBSY on Linux during update (#8468)#8469umago wants to merge 1 commit intoaaif-goose:mainfrom
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 058b88e3eb
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
On Linux, the kernel enforces ETXTBSY ("Text file busy") and refuses to
open a file for writting if that file is being executed.
The original code prior to this change used "fs::copy(...)" which opens
the destination file for writting and this was being blocked by the
ETXTBSY error.
This patch fixes the issue by copying the new binary to a temp file in
the same directory as the current executable and then uses "fs::rename"
to atomically swap the directory entry (it only updates the directory
mapping it doesn't open the file's content).
This basically mimics what is already implemented for Windows in the
update.rs module but adapted for Unix semantics.
Fixes aaif-goose#8468
Signed-off-by: Lucas Alvares Gomes <lucasagomes@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bf9f911c18
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| let tmp_path = dest_dir.join(format!("goose-update-{}", std::process::id())); | ||
|
|
||
| fs::copy(new_binary, &tmp_path) | ||
| .with_context(|| format!("Failed to write update to {}", tmp_path.display()))?; |
There was a problem hiding this comment.
Fresh evidence in this revision: tmp_path is now derived from goose-update-{pid}, which is still deterministic. If a file (or symlink) with that name already exists in the install directory—e.g., from PID reuse, a stale artifact, or a shared writable bin dir—fs::copy will overwrite it and the subsequent rename will remove that entry, causing unintended data loss and potentially writing through a symlink target. Use an atomic create_new/randomized temp filename in dest_dir to guarantee uniqueness before copying.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
It's not deterministic across runs, since different processes ID will be assigned to it. The goose update command is a one-off so, I think it's grand using goose-update-<pid>.
1 participant
Summary
On Linux, the kernel enforces ETXTBSY ("Text file busy") and refuses to open a file for writting if that file is being executed.
The original code prior to this change used "fs::copy(...)" which opens the destination file for writting and this was being blocked by the ETXTBSY error.
This patch fixes the issue by copying the new binary to a temp file in the same directory as the current executable and then uses "fs::rename" to atomically swap the directory entry (it only updates the directory mapping it doesn't open the file's content).
This basically mimics what is already implemented for Windows in the update.rs module but adapted for Unix semantics.
Fixes #8468
Testing
Unit test (test_replace_binary_unix_no_leftover_tmp) and manual test.
Related Issues
Relates to #8468
Discussion: LINK (if any)
Screenshots/Demos (for UX changes)
Before:
$ goose update Downloading goose-x86_64-unknown-linux-gnu.tar.bz2 from stable release... Downloaded 72451366 bytes. Archive SHA-256: 47f5cbc3b55a36fa55b85b7166c606758dd2595137b1c5d246ddf32556a99087 Verifying SLSA provenance via Sigstore... Sigstore provenance verification passed. Error: Failed to replace current binary Caused by: 0: Failed to copy new binary to /home/lmartins/.local/bin/goose 1: Text file busy (os error 26)After: