fix: remap fh_in and fh_out independently for CopyFileRange/RemapFileRange#551
fix: remap fh_in and fh_out independently for CopyFileRange/RemapFileRange#551claude-claude[bot] wants to merge 1 commit intofuse-pipe-restorefrom
Conversation
…Range The previous fix set both fh_in and fh_out to the same value in with_fh(), which corrupts fh_out when only fh_in is stale (or vice versa). Split into independent fh_out()/ino_out()/with_fh_out() helpers so the handle_remap lookup and EBADF retry paths remap each handle separately. Also log a warning when restore_from_table fails to parse inode table JSON instead of silently falling back to an empty table. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
![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: 71341fe6bb
ℹ️ 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".
| if let (Some(old_fh_out), Some(stable_ino_out)) = | ||
| (request.fh_out(), request.ino_out()) | ||
| { | ||
| if let Some(new_fh_out) = | ||
| self.try_reopen_handle(stable_ino_out, old_fh_out, false) |
There was a problem hiding this comment.
fh_out only when it is actually stale
This block unconditionally calls try_reopen_handle for fh_out on every EBADF retry, even when EBADF was caused by fh_in and fh_out is already a valid current handle (a common mixed-handle state after snapshot restore). In that case try_reopen_handle inserts a new mapping for the valid old_fh_out, so later requests are remapped to a duplicate descriptor; when Release arrives, the remapped descriptor is closed and the original valid handle is never released, causing an FD leak that can accumulate over long-running workloads.
Useful? React with 👍 / 👎.
|
Already fixed in latest commits — with_fh() only remaps fh_in now |
1 participant
Auto-Fix for PR #549
Issues Fixed
[MEDIUM]
with_fh()corruptsfh_outfor dual-handle operations — The previous fix (commit13526d1) set bothfh_inandfh_outto the same value inwith_fh(), corrupting the other handle when only one is stale. Nowwith_fh()only updatesfh_in, and newfh_out()/ino_out()/with_fh_out()helpers allow independent remapping. Both the handle_remap lookup and EBADF retry paths now remap each handle separately.[LOW] Silent JSON parse failure in
restore_from_table—serde_json::from_str(json).unwrap_or_default()silently discarded corrupt inode table JSON. Now logs awarn!so operators can diagnose restore failures.Changes
fuse-pipe/src/protocol/request.rs:with_fh()no longer touchesfh_out; addedfh_out(),ino_out(),with_fh_out()helpersfuse-pipe/src/server/remap.rs: Handle remap lookup and EBADF retry now remapfh_outindependently; addedwarnimport and parse failure loggingGenerated by Claude | Review Run