8000 gh-92301: subprocess: Prefer close_range() to procfs-based fd closing… · python/cpython@58573ff · GitHub
[go: up one dir, main page]
Skip to content

Commit 58573ff

Browse files
author
Alexey Izbyshev
authored
gh-92301: subprocess: Prefer close_range() to procfs-based fd closing (#92303)
#92301: subprocess: Prefer `close_range()` to procfs-based fd closing. `close_range()` is much faster for large number of file descriptors, e.g. 4 times faster for 1000 descriptors in a Linux 5.16-based environment. We prefer close_range() only if it's known to be async-signal-safe.
1 parent e65e587 commit 58573ff

File tree

2 files changed

+72
-18
lines changed

2 files changed

+72
-18
lines changed

‎Misc/NEWS.d/next/Library/2022-05-04-11-54-37.gh-issue-92301.eqjoYX.rst

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Prefer ``close_range()`` to iterating over procfs for file descriptor
2+
closing in :mod:`subprocess` for better performance.

‎Modules/_posixsubprocess.c

Lines changed: 70 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -210,18 +210,23 @@ safe_get_max_fd(void)
210210
}
211211

212212

213-
/* Close all file descriptors in the range from start_fd and higher
214-
* except for those in py_fds_to_keep. If the range defined by
215-
* [start_fd, safe_get_max_fd()) is large this will take a long
216-
* time as it calls close() on EVERY possible fd.
213+
/* Close all file descriptors in the given range except for those in
214+
* py_fds_to_keep by invoking closer on each subrange.
217215
*
218-
* It isn't possible to know for sure what the max fd to go up to
219-
* is for processes with the capability of raising their maximum.
216+
* If end_fd == -1, it's guessed via safe_get_max_fd(), but it isn't
217+
* possible to know for sure what the max fd to go up to is for
218+
* processes with the capability of raising their maximum, or in case
219+
* a process opened a high fd and then lowered its maximum.
220220
*/
221-
static void
222-
_close_fds_by_brute_force(long start_fd, PyObject *py_fds_to_keep)
221+
static int
222+
_close_range_except(int start_fd,
223+
int end_fd,
224+
PyObject *py_fds_to_keep,
225+
int (*closer)(int, int))
223226
{
224-
long end_fd = safe_get_max_fd();
227+
if (end_fd == -1) {
228+
end_fd = Py_MIN(safe_get_max_fd(), INT_MAX);
229+
}
225230
Py_ssize_t num_fds_to_keep = PyTuple_GET_SIZE(py_fds_to_keep);
226231
Py_ssize_t keep_seq_idx;
227232
/* As py_fds_to_keep is sorted we can loop through the list closing
@@ -231,15 +236,17 @@ _close_fds_by_brute_force(long start_fd, PyObject *py_fds_to_keep)
231236
int keep_fd = PyLong_AsLong(py_keep_fd);
232237
if (keep_fd < start_fd)
233238
continue;
234-
_Py_closerange(start_fd, keep_fd - 1);
239+
if (closer(start_fd, keep_fd - 1) != 0)
240+
return -1;
235241
start_fd = keep_fd + 1;
236242
}
237243
if (start_fd <= end_fd) {
238-
_Py_closerange(start_fd, end_fd);
244+
if (closer(start_fd, end_fd) != 0)
245+
return -1;
239246
}
247+
return 0;
240248
}
241249

242-
243250
#if defined(__linux__) && defined(HAVE_SYS_SYSCALL_H)
244251
/* It doesn't matter if d_name has room for NAME_MAX chars; we're using this
245252
* only to read a directory of short file descriptor number names. The kernel
@@ -255,6 +262,16 @@ struct linux_dirent64 {
255262
char d_name[256]; /* Filename (null-terminated) */
256263
};
257264

265+
static int
266+
_brute_force_closer(int first, int last)
267+
{
268+
for (int i = first; i <= last; i++) {
269+
/* Ignore errors */
270+
(void)close(i);
271+
}
272+
return 0;
273+
}
274+
258275
/* Close all open file descriptors in the range from start_fd and higher
259276
* Do not close any in the sorted py_fds_to_keep list.
260277
*
@@ -278,7 +295,7 @@ _close_open_fds_safe(int start_fd, PyObject* py_fds_to_keep)
278295
fd_dir_fd = _Py_open_noraise(FD_DIR, O_RDONLY);
279296
if (fd_dir_fd == -1) {
280297
/* No way to get a list of open fds. */
281-
_close_fds_by_brute_force(start_fd, py_fds_to_keep);
298+
_close_range_except(start_fd, -1, py_fds_to_keep, _brute_force_closer);
282299
return;
283300
} else {
284301
char buffer[sizeof(struct linux_dirent64)];
@@ -306,10 +323,16 @@ _close_open_fds_safe(int start_fd, PyObject* py_fds_to_keep)
306323
}
307324
}
308325

309-
#define _close_open_fds _close_open_fds_safe
326+
#define _close_open_fds_fallback _close_open_fds_safe
310327

311328
#else /* NOT (defined(__linux__) && defined(HAVE_SYS_SYSCALL_H)) */
312329

330+
static int
331+
_unsafe_closer(int first, int last)
332+
{
333+
_Py_closerange(first, last);
334+
return 0;
335+
}
313336

314337
/* Close all open file descriptors from start_fd and higher.
315338
* Do not close any in the sorted py_fds_to_keep tuple.
@@ -325,7 +348,7 @@ _close_open_fds_safe(int start_fd, PyObject* py_fds_to_keep)
325348
* http://womble.decadent.org.uk/readdir_r-advisory.html
326349
*/
327350
static void
328-
_close_open_fds_maybe_unsafe(long start_fd, PyObject* py_fds_to_keep)
351+
_close_open_fds_maybe_unsafe(int start_fd, PyObject* py_fds_to_keep)
329352
{
330353
DIR *proc_fd_dir;
331354
#ifndef HAVE_DIRFD
@@ -348,7 +371,7 @@ _close_open_fds_maybe_unsafe(long start_fd, PyObject* py_fds_to_keep)
348371
proc_fd_dir = opendir(FD_DIR);
349372
if (!proc_fd_dir) {
350373
/* No way to get a list of open fds. */
351-
_close_fds_by_brute_force(start_fd, py_fds_to_keep);
374+
_close_range_except(start_fd, -1, py_fds_to_keep, _unsafe_closer);
352375
} else {
353376
struct dirent *dir_entry;
354377
#ifdef HAVE_DIRFD
@@ -369,16 +392,45 @@ _close_open_fds_maybe_unsafe(long start_fd, PyObject* py_fds_to_keep)
369392
}
370393
if (errno) {
371394
/* readdir error, revert behavior. Highly Unlikely. */
372-
_close_fds_by_brute_force(start_fd, py_fds_to_keep);
395+
_close_range_except(start_fd, -1, py_fds_to_keep, _unsafe_closer);
373396
}
374397
closedir(proc_fd_dir);
375398
}
376399
}
377400

378-
#define _close_open_fds _close_open_fds_maybe_unsafe
401+
#define _close_open_fds_fallback _close_open_fds_maybe_unsafe
379402

380403
#endif /* else NOT (defined(__linux__) && defined(HAVE_SYS_SYSCALL_H)) */
381404

405+
/* We can use close_range() library function only if it's known to be
406+
* async-signal-safe.
407+
*
408+
* On Linux, glibc explicitly documents it to be a thin wrapper over
409+
* the system call, and other C libraries are likely to follow glibc.
410+
*/
411+
#if defined(HAVE_CLOSE_RANGE) && \
412+
(defined(__linux__) || defined(__FreeBSD__))
413+
#define HAVE_ASYNC_SAFE_CLOSE_RANGE
414+
415+
static int
416+
_close_range_closer(int first, int last)
417+
{
418+
return close_range(first, last, 0);
419+
}
420+
#endif
421+
422+
static void
423+
_close_open_fds(int start_fd, PyObject* py_fds_to_keep)
424+
{
425+
#ifdef HAVE_ASYNC_SAFE_CLOSE_RANGE
426+
if (_close_range_except(
427+
start_fd, INT_MAX, py_fds_to_keep,
428+
_close_range_closer) == 0) {
429+
return;
430+
}
431+
#endif
432+
_close_open_fds_fallback(start_fd, py_fds_to_keep);
433+
}
382434

383435
#ifdef VFORK_USABLE
384436
/* Reset dispositions for all signals to SIG_DFL except for ignored

0 commit comments

Comments
 (0)
0