Skip to content

read_file masks setup failures and leaks partial resources #424

Description

@OllieinCanada

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.

Activity

  1. PerryLink commented on Sep 13, 2026

    @PerryLink

    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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions