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

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.

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) {
+        _Py_closerange(start_fd, keep_fd - 1);
-            close(fd_num);
-        }
         start_fd = keep_fd + 1;
     }
     if (start_fd <= end_fd) {
-#if defined(__FreeBSD__)
+        _Py_closerange(start_fd, end_fd);
-        /* 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
     }
 }
 

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
+    _Py_closerange(fd_low, fd_high - 1);
-    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_END_SUPPRESS_IPH
     Py_END_ALLOW_THREADS
     Py_RETURN_NONE;

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