diff --git a/Misc/NEWS.d/next/C API/2020-10-10-14-05-24.bpo-40422.sh8IDY.rst b/Misc/NEWS.d/next/C API/2020-10-10-14-05-24.bpo-40422.sh8IDY.rst new file mode 100644 index 00000000000..1b6d9e034b5 --- /dev/null +++ b/Misc/NEWS.d/next/C API/2020-10-10-14-05-24.bpo-40422.sh8IDY.rst @@ -0,0 +1 @@ +Add `_Py_closerange` function to provide performant closing of a range of file descriptors. \ No newline at end of file diff --git a/Modules/_posixsubprocess.c b/Modules/_posixsubprocess.c index 5d1691ace41..ed046fc5c1b 100644 --- a/Modules/_posixsubprocess.c +++ b/Modules/_posixsubprocess.c @@ -250,7 +250,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) { @@ -258,21 +257,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) { -#if defined(__FreeBSD__) - /* Any errors encountered while closing file descriptors are ignored */ - closefrom(start_fd); -#else - for (fd_num = start_fd; fd_num < end_fd; ++fd_num) { - /* Ignore errors */ - (void)close(fd_num); - } -#endif + _Py_closerange(start_fd, end_fd); } } diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index 165625c9a67..321eaec63c4 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -8740,8 +8740,23 @@ os_close_impl(PyObject *module, int fd) Py_RETURN_NONE; } +/* Our selection logic for which function to use is as follows: + * 1. If closefrom(2) is available, we'll attempt to use that next if we're + * closing up to sysconf(_SC_OPEN_MAX). + * 1a. 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. + * 1b. 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) { @@ -8757,7 +8772,36 @@ _fdwalk_close_func(void *lohi, int fd) } return 0; } -#endif /* HAVE_FDWALK */ +#endif /* USE_FDWALK */ + +/* Closes all file descriptors in [first, last], ignoring errors. */ +void +_Py_closerange(int first, int last) +{ + first = Py_MAX(first, 0); +#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 */ +} /*[clinic input] os.closerange @@ -8773,31 +8817,9 @@ static PyObject * os_closerange_impl(PyObject *module, int fd_low, int fd_high) /*[clinic end generated code: output=0ce5c20fcda681c2 input=5855a3d053ebd4ec]*/ { -#ifdef HAVE_FDWALK - int lohi[2]; -#endif Py_BEGIN_ALLOW_THREADS _Py_BEGIN_SUPPRESS_IPH -#ifdef HAVE_FDWALK - lohi[0] = Py_MAX(fd_low, 0); - lohi[1] = fd_high; - fdwalk(_fdwalk_close_func, lohi); -#else - fd_low = Py_MAX(fd_low, 0); -#ifdef __FreeBSD__ - if (fd_high >= sysconf(_SC_OPEN_MAX)) { - /* Any errors encountered while closing file descriptors are ignored */ - closefrom(fd_low); - } - else -#endif - { - for (int i = fd_low; i < fd_high; i++) { - /* Ignore errors */ - (void)close(i); - } - } -#endif + _Py_closerange(fd_low, fd_high - 1); _Py_END_SUPPRESS_IPH Py_END_ALLOW_THREADS Py_RETURN_NONE; diff --git a/Modules/posixmodule.h b/Modules/posixmodule.h index 1e00562abc3..749833f71cd 100644 --- a/Modules/posixmodule.h +++ b/Modules/posixmodule.h @@ -28,6 +28,8 @@ PyAPI_FUNC(int) _Py_Sigset_Converter(PyObject *, void *); #endif /* HAVE_SIGSET_T */ #endif /* Py_LIMITED_API */ +PyAPI_FUNC(void) _Py_closerange(int first, int last); + #ifdef __cplusplus } #endif