[GH-ISSUE #424] read_file masks setup failures and leaks partial resources #243

New issue

Open

opened 2026-10-26 09:47:41 +00:00 by bsakaguchi · 1 comment

bsakaguchi commented

2026-10-26 09:47:41 +00:00

Owner

Originally created by @OllieinCanada on GitHub (Aug 24, 2026).
Original GitHub issue: https://github.com/deepseek-ai/3FS/issues/424

Summary

hf3fs_fuse.io.read_file() assumes every setup step completed when its
finally block runs. If setup fails early, cleanup raises UnboundLocalError,
replaces the useful original exception, and can skip releasing resources that
were already acquired.

Reproduction

The behavior is deterministic without a 3FS mount by replacing the native
binding with a stub:

Make os.open() raise FileNotFoundError.
Call read_file().
Observe UnboundLocalError for the unassigned local fd instead of the
original file error.

The same masking happens when register_fd(), SharedMemory(), make_iovec(),
or make_ioring() fails. In the make_iovec() case, del ior raises before
shm.close() and shm.unlink() run, so the partially created shared memory is
not released by this function.

This is separate from #291's underlying os.symlink() failure: a failure from
that call currently enters this broken cleanup path and can be replaced by an
unrelated UnboundLocalError.

Expected behavior

read_file() should preserve the original setup error and release only the
file descriptor, native registration, iovec/ioring, and shared-memory resources
that were successfully acquired.

The regression can be covered with a host-only Python test using stubbed native
bindings; no 3FS mount, RDMA device, or external service is required.

Originally created by @OllieinCanada on GitHub (Aug 24, 2026). Original GitHub issue: https://github.com/deepseek-ai/3FS/issues/424 ## Summary `hf3fs_fuse.io.read_file()` assumes every setup step completed when its `finally` block runs. If setup fails early, cleanup raises `UnboundLocalError`, replaces the useful original exception, and can skip releasing resources that were already acquired. ## Reproduction The behavior is deterministic without a 3FS mount by replacing the native binding with a stub: 1. Make `os.open()` raise `FileNotFoundError`. 2. Call `read_file()`. 3. Observe `UnboundLocalError` for the unassigned local `fd` instead of the original file error. The same masking happens when `register_fd()`, `SharedMemory()`, `make_iovec()`, or `make_ioring()` fails. In the `make_iovec()` case, `del ior` raises before `shm.close()` and `shm.unlink()` run, so the partially created shared memory is not released by this function. This is separate from #291's underlying `os.symlink()` failure: a failure from that call currently enters this broken cleanup path and can be replaced by an unrelated `UnboundLocalError`. ## Expected behavior `read_file()` should preserve the original setup error and release only the file descriptor, native registration, iovec/ioring, and shared-memory resources that were successfully acquired. The regression can be covered with a host-only Python test using stubbed native bindings; no 3FS mount, RDMA device, or external service is required.

bsakaguchi commented

2026-10-26 09:47:42 +00:00

Author

Owner

@PerryLink commented on GitHub (Sep 13, 2026):

Confirmed on main (hf3fs_fuse/io.py): read_file's finally block unconditionally references fd, ior, iov and shm (deregister_fd(fd) / os.close(fd) / del ior / del iov / shm.close() / shm.unlink()), but all four are assigned only inside the try. If os.open() raises, the finally runs with fd unassigned - UnboundLocalError replaces the original error; if make_iovec() fails, del ior raises before shm.close()/shm.unlink() run, leaking the SharedMemory segment. Your stub-based repro is the right shape and a host-only test needs no mount. The fix is to track each acquired resource (or default them upfront) and release only what was actually acquired. The regression also connects to #291: its os.symlink() failure enters this same broken cleanup path.

@PerryLink commented on GitHub (Sep 13, 2026): Confirmed on main (hf3fs_fuse/io.py): read_file's finally block unconditionally references fd, ior, iov and shm (deregister_fd(fd) / os.close(fd) / del ior / del iov / shm.close() / shm.unlink()), but all four are assigned only inside the try. If os.open() raises, the finally runs with fd unassigned - UnboundLocalError replaces the original error; if make_iovec() fails, `del ior` raises before shm.close()/shm.unlink() run, leaking the SharedMemory segment. Your stub-based repro is the right shape and a host-only test needs no mount. The fix is to track each acquired resource (or default them upfront) and release only what was actually acquired. The regression also connects to #291: its os.symlink() failure enters this same broken cleanup path.

bsakaguchi referenced this issue

2026-10-26 09:48:50 +00:00

[PR #243] [CLOSED] PioV.cc: Delete an unused parameter #339