Skip to content

Commit b01a091

Browse files
jg-heotohtana
andauthored
fix(io): close aio_fd in FastFileWriter._fini to prevent fd leak (#8005)
Fixes #8003 ## Summary `FastFileWriter._fini()` overwrote `self._aio_fd = INVALID_FD` without calling `os.close()`, leaking one fd per save. With unlink-based checkpoint rotation this stranded the unlinked inode in the ext4 orphan list, fs blocks were never reclaimed, and long-running save loops hit ENOSPC at iter ~60 (60 GB/iter on a 4 TB partition). This PR adds explicit `os.fsync()` + `os.close()` in `_fini()` and a regression test that asserts no `/proc/self/fd` entry points at a deleted file after a save+close+unlink cycle. ## Verification - 20-iteration repro of `save() / close() / unlink()` leaked 20 fds before the fix, 0 after. - 700-iter / 42 TB / 60 h endurance run on ext4/NVMe: `df_used` stable at 736 GB (drift +281 MB / 697 rotations) with the fix; same workload hit ENOSPC at iter ~60 without it. - Performance impact: ~5% wall-time overhead from the added `os.fsync()` at ~10 GB/s peak. ## Test plan - [x] New regression test `tests/unit/ops/aio/test_fast_file_writer_fd_close.py` verifies fd cleanup after a single save and after 5/20-iter rotation loops via `/proc/self/fd` scoped to `tmp_path`. - [x] Gated on `async_io` compatibility, Linux, and CUDA accelerator so unsupported CI matrix entries skip cleanly. - [x] Confirmed test FAILS without this PR's `_fini()` change and PASSES with it. - [x] `pre-commit run --files <changed files>` clean. ## Notes - The `__del__` assertion `assert self._aio_fd == INVALID_FD` passes even with the bug because it checks the Python attribute that `_fini` itself sets. The new test checks OS-level state via `/proc/self/fd`. - `os.fsync()` is included for post-close durability — required for correctness on the unaligned-tail path that re-opens the file as buffered I/O. If maintainers prefer to drop it for performance, removing only the `os.fsync(...)` line still fixes the leak. Happy to adjust shape, naming, or test placement to fit project conventions. Thanks for the review. Signed-off-by: jg-heo <csjg.heo@gmail.com> Co-authored-by: Masahiro Tanaka <81312776+tohtana@users.noreply.github.com>
1 parent 66af8f0 commit b01a091

2 files changed

Lines changed: 113 additions & 0 deletions

File tree

deepspeed/io/fast_file_writer.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,13 @@ def _fini(self):
114114
if not self._io_buffer_is_empty():
115115
self._force_drain()
116116
self._io_buffer.reset()
117+
fd = self._aio_fd
117118
self._aio_fd = INVALID_FD
119+
if fd != INVALID_FD:
120+
try:
121+
os.fsync(fd)
122+
finally:
123+
os.close(fd)
118124

119125
def _fill_io_buffer(self, src_tensor, src_offset):
120126
st = time.time()
Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,107 @@
1+
# Copyright (c) Microsoft Corporation.
2+
# SPDX-License-Identifier: Apache-2.0
3+
4+
# DeepSpeed Team
5+
"""Regression test for FastFileWriter file-descriptor cleanup.
6+
7+
Without proper os.close() in _fini(), every save through FastFileWriter
8+
leaks one fd pointing at the just-written file. When the user later
9+
unlinks the file (e.g. checkpoint rotation), the leaked fd holds the
10+
inode in the filesystem's orphan list, so blocks are never freed and
11+
the filesystem eventually reports ENOSPC even though `ls` shows only
12+
N files on disk.
13+
14+
These tests assert that after FastFileWriter.close() returns, no fd
15+
in /proc/self/fd points at the (possibly already-unlinked) file.
16+
"""
17+
import os
18+
import sys
19+
import pytest
20+
import torch
21+
22+
import deepspeed
23+
from deepspeed.accelerator import get_accelerator
24+
from deepspeed.ops.op_builder import AsyncIOBuilder
25+
from deepspeed.io import FastFileWriter, FastFileWriterConfig
26+
27+
if not deepspeed.ops.__compatible_ops__[AsyncIOBuilder.NAME]:
28+
pytest.skip("async_io op is not compatible on this system", allow_module_level=True)
29+
30+
if not sys.platform.startswith("linux"):
31+
pytest.skip("test uses /proc/self/fd which is Linux-only", allow_module_level=True)
32+
33+
if get_accelerator().device_name() != 'cuda':
34+
pytest.skip("FastFileWriter requires CUDA-pinned tensors", allow_module_level=True)
35+
36+
BLOCK_SIZE = 1 * 1024 * 1024
37+
QUEUE_DEPTH = 8
38+
PINNED_BYTES = 8 * 1024 * 1024
39+
PAYLOAD_BYTES = 1 * 1024 * 1024
40+
41+
42+
def _count_deleted_fds(target_dir):
43+
"""How many fds in /proc/self/fd point at a now-deleted file located
44+
under target_dir? Restricting to target_dir avoids false positives
45+
from unrelated deleted fds in the test process."""
46+
pid = os.getpid()
47+
n = 0
48+
for entry in os.listdir(f"/proc/{pid}/fd"):
49+
try:
50+
target = os.readlink(f"/proc/{pid}/fd/{entry}")
51+
except OSError:
52+
continue
53+
if target.startswith(str(target_dir)) and target.endswith("(deleted)"):
54+
n += 1
55+
return n
56+
57+
58+
def _build_writer(file_path):
59+
aio = AsyncIOBuilder().load(verbose=False).aio_handle(block_size=BLOCK_SIZE,
60+
queue_depth=QUEUE_DEPTH,
61+
single_submit=False,
62+
overlap_events=False,
63+
intra_op_parallelism=1)
64+
pinned = torch.zeros(PINNED_BYTES, dtype=torch.uint8).pin_memory()
65+
cfg = FastFileWriterConfig(dnvme_handle=aio,
66+
pinned_tensor=pinned,
67+
double_buffer=True,
68+
num_parallel_writers=1,
69+
writer_rank=0)
70+
return FastFileWriter(file_path=str(file_path), config=cfg)
71+
72+
73+
def test_close_releases_fd_after_unlink(tmp_path):
74+
"""Single save + unlink must not leave a deleted-fd reference."""
75+
target = tmp_path / "ckpt_single.pt"
76+
buf = torch.zeros(PAYLOAD_BYTES, dtype=torch.uint8)
77+
78+
before = _count_deleted_fds(tmp_path)
79+
w = _build_writer(target)
80+
torch.save(obj=buf, f=w)
81+
w.close()
82+
os.unlink(target)
83+
after = _count_deleted_fds(tmp_path)
84+
85+
assert after == before, (f"FastFileWriter leaked an fd: deleted-fd count went "
86+
f"from {before} to {after} after a single save+close+unlink. "
87+
f"This indicates _fini() did not os.close(self._aio_fd).")
88+
89+
90+
@pytest.mark.parametrize("n_iters", [5, 20])
91+
def test_rotation_loop_does_not_leak(tmp_path, n_iters):
92+
"""N iterations of save+close+unlink should leave zero deleted-fds.
93+
Mirrors the real checkpoint-rotation workload that originally
94+
surfaced this bug."""
95+
buf = torch.zeros(PAYLOAD_BYTES, dtype=torch.uint8)
96+
before = _count_deleted_fds(tmp_path)
97+
98+
for i in range(n_iters):
99+
path = tmp_path / f"ckpt_{i}.pt"
100+
w = _build_writer(path)
101+
torch.save(obj=buf, f=w)
102+
w.close()
103+
os.unlink(path)
104+
105+
after = _count_deleted_fds(tmp_path)
106+
assert after == before, (f"FastFileWriter leaked {after - before} fd(s) over {n_iters} "
107+
f"save+close+unlink iterations (expected 0).")

0 commit comments

Comments
 (0)