diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index 3cf0683ad9..fb070e4cc4 100644 --- Modules/_posixsubprocess.c +++ Modules/_posixsubprocess.c @@ -21,6 +21,8 @@ #include #endif +#include "posixmodule.h" + #ifdef _Py_MEMORY_SANITIZER # include #endif @@ -222,7 +222,6 @@ _close_fds_by_brute_force(long start_fd, PyObject *py_fds_to_keep) long end_fd = safe_get_max_fd(); Py_ssize_t num_fds_to_keep = PyTuple_GET_SIZE(py_fds_to_keep); Py_ssize_t keep_seq_idx; - int fd_num; /* As py_fds_to_keep is sorted we can loop through the list closing * fds in between any in the keep list falling within our range. */ for (keep_seq_idx = 0; keep_seq_idx < num_fds_to_keep; ++keep_seq_idx) { @@ -230,15 +229,11 @@ _close_fds_by_brute_force(long start_fd, PyObject *py_fds_to_keep) int keep_fd = PyLong_AsLong(py_keep_fd); if (keep_fd < start_fd) continue; - for (fd_num = start_fd; fd_num < keep_fd; ++fd_num) { - close(fd_num); - } + _Py_closerange(start_fd, keep_fd - 1); start_fd = keep_fd + 1; } if (start_fd <= end_fd) { - for (fd_num = start_fd; fd_num < end_fd; ++fd_num) { - close(fd_num); - } + _Py_closerange(start_fd, end_fd); } } diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 43d4302b92..7878517353 100644 --- Modules/posixmodule.c +++ Modules/posixmodule.c @@ -7820,6 +7820,78 @@ os_close_impl(PyObject *module, int fd) Py_RETURN_NONE; } +/* Our selection logic for which function to use is as follows: + * 1. If close_range(2) is available, always prefer that; it's better for + * contiguous ranges like this than fdwalk(3) which entails iterating over + * the entire fd space and simply doing nothing for those outside the range. + * 2. If closefrom(2) is available, we'll attempt to use that next if we're + * closing up to sysconf(_SC_OPEN_MAX). + * 2a. Fallback to fdwalk(3) if we're not closing up to sysconf(_SC_OPEN_MAX), + * as that will be more performant if the range happens to have any chunk of + * non-opened fd in the middle. + * 2b. If fdwalk(3) isn't available, just do a plain close(2) loop. + */ +#ifdef __FreeBSD__ +#define USE_CLOSEFROM +#endif /* __FreeBSD__ */ + +#ifdef HAVE_FDWALK +#define USE_FDWALK +#endif /* HAVE_FDWALK */ + +#ifdef USE_FDWALK +static int +_fdwalk_close_func(void *lohi, int fd) +{ + int lo = ((int *)lohi)[0]; + int hi = ((int *)lohi)[1]; + + if (fd >= hi) + return 1; + else if (fd >= lo) + close(fd); + return 0; +} +#endif /* USE_FDWALK */ + +/* Closes all file descriptors in [first, last], ignoring errors. */ +void +_Py_closerange(int first, int last) +{ + first = Py_MAX(first, 0); + _Py_BEGIN_SUPPRESS_IPH +#ifdef HAVE_CLOSE_RANGE + if (close_range(first, last, 0) == 0 || errno != ENOSYS) { + /* Any errors encountered while closing file descriptors are ignored; + * ENOSYS means no kernel support, though, + * so we'll fallback to the other methods. */ + } + else +#endif /* HAVE_CLOSE_RANGE */ +#ifdef USE_CLOSEFROM + if (last >= sysconf(_SC_OPEN_MAX)) { + /* Any errors encountered while closing file descriptors are ignored */ + closefrom(first); + } + else +#endif /* USE_CLOSEFROM */ +#ifdef USE_FDWALK + { + int lohi[2]; + lohi[0] = first; + lohi[1] = last + 1; + fdwalk(_fdwalk_close_func, lohi); + } +#else + { + for (int i = first; i <= last; i++) { + /* Ignore errors */ + (void)close(i); + } + } +#endif /* USE_FDWALK */ + _Py_END_SUPPRESS_IPH +} /*[clinic input] os.closerange @@ -7835,12 +7907,8 @@ static PyObject * os_closerange_impl(PyObject *module, int fd_low, int fd_high) /*[clinic end generated code: output=0ce5c20fcda681c2 input=5855a3d053ebd4ec]*/ { - int i; Py_BEGIN_ALLOW_THREADS - _Py_BEGIN_SUPPRESS_IPH - for (i = Py_MAX(fd_low, 0); i < fd_high; i++) - close(i); - _Py_END_SUPPRESS_IPH + _Py_closerange(fd_low, fd_high - 1); Py_END_ALLOW_THREADS Py_RETURN_NONE; } diff --git a/Modules/posixmodule.h b/Modules/posixmodule.h index 1ec1833825..be2308ea86 100644 --- Modules/posixmodule.h +++ Modules/posixmodule.h @@ -19,6 +19,8 @@ PyAPI_FUNC(int) _Py_Gid_Converter(PyObject *, void *); #endif /* MS_WINDOWS */ #endif +PyAPI_FUNC(void) _Py_closerange(int first, int last); + #ifdef __cplusplus } #endif diff --git a/configure b/configure index 829dd69bb8..2eeadec5f0 100755 --- configure +++ configure @@ -11490,9 +11490,9 @@ fi # checks for library functions for ac_func in alarm accept4 setitimer getitimer bind_textdomain_codeset chown \ - clock confstr ctermid dup3 execv faccessat fchmod fchmodat fchown fchownat \ - fexecve fdopendir fork fpathconf fstatat ftime ftruncate futimesat \ - futimens futimes gai_strerror getentropy \ + clock close_range confstr ctermid dup3 execv faccessat fchmod fchmodat fchown \ + fchownat fdwalk fexecve fdopendir fork fpathconf fstatat ftime ftruncate \ + futimesat futimens futimes gai_strerror getentropy \ getgrouplist getgroups getlogin getloadavg getpeername getpgid getpid \ getpriority getresuid getresgid getpwent getspnam getspent getsid getwd \ if_nameindex \ diff --git a/configure.ac b/configure.ac index f1cc8e9bcb..80952290b7 100644 --- configure.ac +++ configure.ac @@ -3574,9 +3574,9 @@ fi # checks for library functions AC_CHECK_FUNCS(alarm accept4 setitimer getitimer bind_textdomain_codeset chown \ - clock confstr ctermid dup3 execv faccessat fchmod fchmodat fchown fchownat \ - fexecve fdopendir fork fpathconf fstatat ftime ftruncate futimesat \ - futimens futimes gai_strerror getentropy \ + clock close_range confstr ctermid dup3 execv faccessat fchmod fchmodat fchown \ + fchownat fdwalk fexecve fdopendir fork fpathconf fstatat ftime ftruncate \ + futimesat futimens futimes gai_strerror getentropy \ getgrouplist getgroups getlogin getloadavg getpeername getpgid getpid \ getpriority getresuid getresgid getpwent getspnam getspent getsid getwd \ if_nameindex \ diff --git a/pyconfig.h.in b/pyconfig.h.in index ebab5ff518..e1d659059c 100644 --- pyconfig.h.in +++ pyconfig.h.in @@ -128,6 +128,9 @@ /* Define to 1 if you have the `clock_settime' function. */ #undef HAVE_CLOCK_SETTIME +/* Define to 1 if you have the `close_range' function. */ +#undef HAVE_CLOSE_RANGE + /* Define if the C compiler supports computed gotos. */ #undef HAVE_COMPUTED_GOTOS @@ -324,6 +327,9 @@ /* Define to 1 if you have the `fdopendir' function. */ #undef HAVE_FDOPENDIR +/* Define to 1 if you have the `fdwalk' function. */ +#undef HAVE_FDWALK + /* Define to 1 if you have the `fexecve' function. */ #undef HAVE_FEXECVE