bpo-40422: create a common _Py_closerange API (GH-19754)

Such an API can be used both for os.closerange and subprocess. For the latter, this yields potential improvement for platforms that have fdwalk but wouldn't have used it there. This will prove even more beneficial later for platforms that have close_range(2), as the new API will prefer that over all else if it's available.

The new API is structured to look more like close_range(2), closing from [start, end] rather than the [low, high) of os.closerange().

Automerge-Triggered-By: @gpshead

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 (file)
index 0000000..1b6d9e0
--- /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 5d1691ace4192003a1449749998e3732fc83fc44..ed046fc5c1ba9f39575d64c45423e1820e52ca91 100644 (file)
--- 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 165625c9a670a5cb961280acfd68ed71e8afec4f..321eaec63c4a489e446f83021cbc832af5c098e5 100644 (file)
--- 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 1e00562abc3370eb44631b6c065f47e2d995fdd7..749833f71cd4de5ab420ebdbc60eeaf4594d65d0 100644 (file)
--- 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