From 35d98b3175905974bf6e1425a718d49e789c938c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EC=9D=B4=EA=B0=95=EC=9D=80?= Date: Sun, 6 Sep 2026 20:34:14 +0900 Subject: [PATCH 1/4] List the fd directory in close_open_fds() instead of walking the limit #455 made close_open_fds() call os.closerange() on the gaps between kept descriptors, which is close_range(2) on Linux and finishes in ~0 ms whatever RLIMIT_NOFILE says. Where that syscall is unavailable (seccomp denying it, kernels before 5.9, macOS) os.closerange() falls back to a C loop over the range, still about 3 minutes at the container limit from celery/celery#9886. List /proc/self/fd (or /dev/fd on macOS, and on FreeBSD/DragonFly when fdescfs is mounted) and close only the descriptors actually open, the way CPython's subprocess does; keep the closerange() path as the fallback. get_fdmax() is no longer consulted on the fd directory path, so its value doesn't need capping (cf. #417). Errors from os.close() are ignored, matching os.closerange(). --- billiard/compat.py | 50 +++++++++++++++++++++ t/unit/test_compat.py | 102 +++++++++++++++++++++++++++++++++++++++--- 2 files changed, 145 insertions(+), 7 deletions(-) diff --git a/billiard/compat.py b/billiard/compat.py index b75455a..4f3b405 100644 --- a/billiard/compat.py +++ b/billiard/compat.py @@ -117,11 +117,61 @@ def uniq(it): return (seen.add(obj) or obj for obj in it if obj not in seen) +# Directory listing the descriptors open in the calling process. Same +# choice as CPython's FD_DIR (Modules/_posixsubprocess.c): /dev/fd on +# macOS, FreeBSD and DragonFly, /proc/self/fd elsewhere. On FreeBSD and +# DragonFly it is only trusted when fdescfs is mounted, see _open_fds(). +_BSD_WITH_FDESCFS = ('freebsd', 'dragonfly') +if sys.platform == 'darwin' or sys.platform.startswith(_BSD_WITH_FDESCFS): + _FD_DIR = '/dev/fd' +else: + _FD_DIR = '/proc/self/fd' + + +def _dev_fd_is_fdescfs(): + # devfs alone creates only /dev/fd/0-2, while fdescfs creates entries for + # every descriptor the process has open. Same check as CPython's + # _is_fdescfs_mounted_on_dev_fd(). + try: + return os.stat('/dev').st_dev != os.stat(_FD_DIR).st_dev + except OSError: + return False + + +def _open_fds(): + """Return the descriptors open in this process, or None. + + None means this platform has no directory listing them, and the caller + has to fall back to a numeric range. + """ + if sys.platform.startswith(_BSD_WITH_FDESCFS) and not _dev_fd_is_fdescfs(): + return None + try: + names = os.listdir(_FD_DIR) + except OSError: + return None + return sorted(int(name) for name in names if name.isdigit()) + + def close_open_fds(keep=None): # must make sure this is 0-inclusive (Issue #celery/1882) keep = list(uniq(sorted( f for f in map(maybe_fileno, keep or []) if f is not None ))) + fds = _open_fds() + if fds is not None: + # Only the descriptors actually open are touched, so the cost does + # not depend on RLIMIT_NOFILE (celery/celery#9886). + keep = set(keep) + for fd in fds: + if fd in keep: + continue + try: + os.close(fd) + except OSError: + # Same as os.closerange() below: closing is best effort. + pass + return maxfd = get_fdmax(default=2048) kL, kH = iter([-1] + keep), iter(keep + [maxfd]) for low, high in zip_longest(kL, kH): diff --git a/t/unit/test_compat.py b/t/unit/test_compat.py index 2def862..c79b41a 100644 --- a/t/unit/test_compat.py +++ b/t/unit/test_compat.py @@ -1,12 +1,59 @@ +import errno +import sys +import tempfile from unittest.mock import Mock, call, patch +import pytest + +from billiard import compat from billiard.compat import close_open_fds class test_close_open_fds: + def test_closes_only_listed_descriptors(self): + with patch('billiard.compat._open_fds', + return_value=[0, 1, 2, 7, 9]), \ + patch('os.close') as close, \ + patch('os.closerange') as closerange: + close_open_fds([1, 2]) + assert close.call_args_list == [call(0), call(7), call(9)] + closerange.assert_not_called() + + def test_accepts_file_objects_and_ignores_none(self): + fh = Mock(name='fh') + fh.fileno.return_value = 3 + with patch('billiard.compat._open_fds', return_value=[3, 4]), \ + patch('os.close') as close, \ + patch('os.closerange'): + close_open_fds([None, fh]) + close.assert_called_once_with(4) + + @pytest.mark.parametrize('code', [errno.EBADF, errno.EINTR, errno.EIO]) + def test_ignores_close_errors_like_closerange(self, code): + exc = OSError() + exc.errno = code + with patch('billiard.compat._open_fds', return_value=[5, 6]), \ + patch('os.close', side_effect=exc) as close, \ + patch('os.closerange'): + close_open_fds() + assert close.call_args_list == [call(5), call(6)] + + def test_never_calls_get_fdmax_with_fd_dir(self): + """The limit is irrelevant when the open descriptors can be listed.""" + with patch('billiard.compat._open_fds', return_value=[0, 1, 2]), \ + patch('billiard.compat.get_fdmax') as get_fdmax, \ + patch('os.close'): + close_open_fds([0, 1, 2]) + get_fdmax.assert_not_called() + + +class test_close_open_fds_fallback: + """Without an fd directory the gaps between kept descriptors are closed.""" + def test_closes_ranges_between_kept_descriptors(self): - with patch('billiard.compat.get_fdmax', return_value=10), \ + with patch('billiard.compat._open_fds', return_value=None), \ + patch('billiard.compat.get_fdmax', return_value=10), \ patch('os.closerange') as closerange, \ patch('os.close') as close: close_open_fds([1, 2, 5]) @@ -18,14 +65,16 @@ def test_closes_ranges_between_kept_descriptors(self): def test_accepts_file_objects_and_ignores_none(self): fh = Mock(name='fh') fh.fileno.return_value = 4 - with patch('billiard.compat.get_fdmax', return_value=6), \ + with patch('billiard.compat._open_fds', return_value=None), \ + patch('billiard.compat.get_fdmax', return_value=6), \ patch('os.closerange') as closerange, \ patch('os.close'): close_open_fds([None, fh, 4, 0]) assert closerange.call_args_list == [call(1, 4), call(5, 6)] def test_without_keep_closes_whole_range(self): - with patch('billiard.compat.get_fdmax', return_value=3), \ + with patch('billiard.compat._open_fds', return_value=None), \ + patch('billiard.compat.get_fdmax', return_value=3), \ patch('os.closerange') as closerange, \ patch('os.close'): close_open_fds() @@ -34,13 +83,52 @@ def test_without_keep_closes_whole_range(self): def test_cost_does_not_grow_with_fdmax(self): """One closerange() call per gap, however large the limit is. - The previous body called os.close() once per descriptor up to - get_fdmax(), which stalls for minutes in containers where the limit - is ~1e9 (celery/celery#9886). + The body before celery/billiard#455 called os.close() once per + descriptor up to get_fdmax(), which stalls for minutes in containers + where the limit is ~1e9 (celery/celery#9886). """ - with patch('billiard.compat.get_fdmax', return_value=2 ** 20), \ + with patch('billiard.compat._open_fds', return_value=None), \ + patch('billiard.compat.get_fdmax', return_value=2 ** 20), \ patch('os.closerange') as closerange, \ patch('os.close') as close: close_open_fds([0, 1, 2]) closerange.assert_called_once_with(3, 2 ** 20) close.assert_not_called() + + +class test_open_fds: + + def test_fd_dir_matches_this_platform(self): + if sys.platform == 'darwin': + assert compat._FD_DIR == '/dev/fd' + elif sys.platform.startswith(('freebsd', 'dragonfly')): + assert compat._FD_DIR == '/dev/fd' + else: + assert compat._FD_DIR == '/proc/self/fd' + + @pytest.mark.skipif(sys.platform == 'win32', reason='no fd directory') + def test_lists_a_descriptor_this_process_opened(self): + with tempfile.TemporaryFile() as fh: + fds = compat._open_fds() + assert fds is not None + assert fh.fileno() in fds + + def test_returns_none_when_fd_dir_is_missing(self): + with patch('os.listdir', side_effect=OSError()): + assert compat._open_fds() is None + + def test_returns_none_on_freebsd_without_fdescfs(self): + with patch.object(sys, 'platform', 'freebsd14'), \ + patch('billiard.compat._dev_fd_is_fdescfs', + return_value=False), \ + patch('os.listdir') as listdir: + assert compat._open_fds() is None + listdir.assert_not_called() + + def test_dev_fd_is_fdescfs(self): + with patch('os.stat', side_effect=[Mock(st_dev=1), Mock(st_dev=2)]): + assert compat._dev_fd_is_fdescfs() + with patch('os.stat', side_effect=[Mock(st_dev=1), Mock(st_dev=1)]): + assert not compat._dev_fd_is_fdescfs() + with patch('os.stat', side_effect=OSError()): + assert not compat._dev_fd_is_fdescfs() From 85456e448b1642b9a4ba37e62d969ae21a26a3c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Asif=20Saif=20Uddin=20=7B=22Auvi=22=3A=22=E0=A6=85?= =?UTF-8?q?=E0=A6=AD=E0=A6=BF=22=7D?= Date: Sun, 6 Sep 2026 20:19:18 +0600 Subject: [PATCH 2/4] Handle missing fd directory in test Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- t/unit/test_compat.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/t/unit/test_compat.py b/t/unit/test_compat.py index c79b41a..380fd7c 100644 --- a/t/unit/test_compat.py +++ b/t/unit/test_compat.py @@ -110,7 +110,8 @@ def test_fd_dir_matches_this_platform(self): def test_lists_a_descriptor_this_process_opened(self): with tempfile.TemporaryFile() as fh: fds = compat._open_fds() - assert fds is not None + if fds is None: + pytest.skip(f'fd directory {compat._FD_DIR!r} not available on this system') assert fh.fileno() in fds def test_returns_none_when_fd_dir_is_missing(self): From 2c86166c060a5ae9ee514ed20083d3898788d081 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Asif=20Saif=20Uddin=20=7B=22Auvi=22=3A=22=E0=A6=85?= =?UTF-8?q?=E0=A6=AD=E0=A6=BF=22=7D?= Date: Sun, 6 Sep 2026 20:25:41 +0600 Subject: [PATCH 3/4] Apply batched suggestions from code review Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> --- billiard/compat.py | 2 +- t/unit/test_compat.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/billiard/compat.py b/billiard/compat.py index 4f3b405..25d3adc 100644 --- a/billiard/compat.py +++ b/billiard/compat.py @@ -122,7 +122,7 @@ def uniq(it): # macOS, FreeBSD and DragonFly, /proc/self/fd elsewhere. On FreeBSD and # DragonFly it is only trusted when fdescfs is mounted, see _open_fds(). _BSD_WITH_FDESCFS = ('freebsd', 'dragonfly') -if sys.platform == 'darwin' or sys.platform.startswith(_BSD_WITH_FDESCFS): +if sys.platform.startswith(('cygwin', 'darwin') + _BSD_WITH_FDESCFS): _FD_DIR = '/dev/fd' else: _FD_DIR = '/proc/self/fd' diff --git a/t/unit/test_compat.py b/t/unit/test_compat.py index 380fd7c..6c48768 100644 --- a/t/unit/test_compat.py +++ b/t/unit/test_compat.py @@ -101,7 +101,7 @@ class test_open_fds: def test_fd_dir_matches_this_platform(self): if sys.platform == 'darwin': assert compat._FD_DIR == '/dev/fd' - elif sys.platform.startswith(('freebsd', 'dragonfly')): + elif sys.platform.startswith(('cygwin', 'freebsd', 'dragonfly')): assert compat._FD_DIR == '/dev/fd' else: assert compat._FD_DIR == '/proc/self/fd' From ece37f1825f266b2aa523258bcecdb3487f44710 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=EC=9D=B4=EA=B0=95=EC=9D=80?= Date: Mon, 7 Sep 2026 12:54:39 +0900 Subject: [PATCH 4/4] Patch os.closerange in the remaining fd-directory test Also wrap the skip message from 85456e4 for flake8 and note gh-148575 in the FD_DIR comment, since Cygwin is only there from that change on. --- billiard/compat.py | 7 ++++--- t/unit/test_compat.py | 7 +++++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/billiard/compat.py b/billiard/compat.py index 25d3adc..1324166 100644 --- a/billiard/compat.py +++ b/billiard/compat.py @@ -118,9 +118,10 @@ def uniq(it): # Directory listing the descriptors open in the calling process. Same -# choice as CPython's FD_DIR (Modules/_posixsubprocess.c): /dev/fd on -# macOS, FreeBSD and DragonFly, /proc/self/fd elsewhere. On FreeBSD and -# DragonFly it is only trusted when fdescfs is mounted, see _open_fds(). +# choice as CPython's FD_DIR (Modules/_posixsubprocess.c, Cygwin added in +# gh-148575): /dev/fd on macOS, Cygwin, FreeBSD and DragonFly, /proc/self/fd +# elsewhere. On FreeBSD and DragonFly it is only trusted when fdescfs is +# mounted, see _open_fds(). _BSD_WITH_FDESCFS = ('freebsd', 'dragonfly') if sys.platform.startswith(('cygwin', 'darwin') + _BSD_WITH_FDESCFS): _FD_DIR = '/dev/fd' diff --git a/t/unit/test_compat.py b/t/unit/test_compat.py index 6c48768..040553a 100644 --- a/t/unit/test_compat.py +++ b/t/unit/test_compat.py @@ -43,9 +43,11 @@ def test_never_calls_get_fdmax_with_fd_dir(self): """The limit is irrelevant when the open descriptors can be listed.""" with patch('billiard.compat._open_fds', return_value=[0, 1, 2]), \ patch('billiard.compat.get_fdmax') as get_fdmax, \ - patch('os.close'): + patch('os.close'), \ + patch('os.closerange') as closerange: close_open_fds([0, 1, 2]) get_fdmax.assert_not_called() + closerange.assert_not_called() class test_close_open_fds_fallback: @@ -111,7 +113,8 @@ def test_lists_a_descriptor_this_process_opened(self): with tempfile.TemporaryFile() as fh: fds = compat._open_fds() if fds is None: - pytest.skip(f'fd directory {compat._FD_DIR!r} not available on this system') + pytest.skip( + f'fd directory {compat._FD_DIR!r} not available here') assert fh.fileno() in fds def test_returns_none_when_fd_dir_is_missing(self):