diff -r 110b95446ec2 Include/setcloexec.h --- /dev/null Thu Jan 01 00:00:00 1970 +0000 +++ b/Include/setcloexec.h Thu Jun 12 12:54:32 2014 -0700 @@ -0,0 +1,63 @@ +#ifndef Py_LIMITED_API +#ifndef Py_SETCLOEXEC_H +#define Py_SETCLOEXEC_H + +#include + +#ifdef HAVE_SYS_IOCTL_H +#include +#endif + +#ifdef __cplusplus +extern "C" { +#endif + +/* + * Sets the CLOEXEC flag on the file descriptor. + * + * Unix-like specific. Safe to use after a fork or other asynch signal + * situation. + */ +static int _Py_setcloexec(int fd, int inheritable); + +/* + * This is just inline to simplify the build and because it is really + * tiny code anyways. Also often inheritable is a constant so the code + * can be smaller than what one might think. + */ +#if defined(HAVE_SYS_IOCTL_H) && defined(FIOCLEX) && defined(FIONCLEX) +static inline int _Py_setcloexec(int fd, int inheritable) +{ + int request; + + if (inheritable) + request = FIONCLEX; + else + request = FIOCLEX; + + return ioctl(fd, request, NULL); +} +#else +static inline int _Py_setcloexec(int fd, int inheritable) +{ + int flags; + + flags = fcntl(fd, F_GETFD); + if (flags < 0) { + return -1; + } + + if (inheritable) + flags &= ~FD_CLOEXEC; + else + flags |= FD_CLOEXEC; + return fcntl(fd, F_SETFD, flags); +} +#endif + +#ifdef __cplusplus +} +#endif + +#endif +#endif /* !Py_LIMITED_API */ diff -r 110b95446ec2 Modules/_posixsubprocess.c --- a/Modules/_posixsubprocess.c Mon Jun 02 00:31:44 2014 +0100 +++ b/Modules/_posixsubprocess.c Thu Jun 12 12:54:32 2014 -0700 @@ -1,5 +1,8 @@ /* Authors: Gregory P. Smith & Jeffrey Yasskin */ #include "Python.h" + +#include "setcloexec.h" + #if defined(HAVE_PIPE2) && !defined(_GNU_SOURCE) # define _GNU_SOURCE #endif @@ -182,17 +185,26 @@ return local_max_fd; } +static void +setcloexec(int fd) +{ + if (-1 == _Py_setcloexec(fd, true)) { + int errnum = errno; + assert(errnum != EINVAL); + /* The error may be EBADF for the brute force usage */ + } +} -/* Close all file descriptors in the range from start_fd and higher - * except for those in py_fds_to_keep. If the range defined by - * [start_fd, safe_get_max_fd()) is large this will take a long - * time as it calls close() on EVERY possible fd. +/* Set to close on exec all file descriptors in the range from + * start_fd and higher except for those in py_fds_to_keep. If the + * range defined by [start_fd, safe_get_max_fd()) is large this will + * take a long time as it calls fcntl() on EVERY possible fd. * * It isn't possible to know for sure what the max fd to go up to * is for processes with the capability of raising their maximum. */ static void -_close_fds_by_brute_force(long start_fd, PyObject *py_fds_to_keep) +_setcloexec_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 = PySequence_Length(py_fds_to_keep); @@ -213,7 +225,7 @@ } if (start_fd <= end_fd) { for (fd_num = start_fd; fd_num < end_fd; ++fd_num) { - while (close(fd_num) < 0 && errno == EINTR); + setcloexec(fd_num); } } } @@ -234,8 +246,9 @@ char d_name[256]; /* Filename (null-terminated) */ }; -/* Close all open file descriptors in the range from start_fd and higher - * Do not close any in the sorted py_fds_to_keep list. +/* Set to close on exec all open file descriptors in the range from + * start_fd and higher. Do not set to close any in the sorted + * py_fds_to_keep list. * * This version is async signal safe as it does not make any unsafe C library * calls, malloc calls or handle any locks. It is _unfortunate_ to be forced @@ -250,14 +263,14 @@ * it with some cpp #define magic to work on other OSes as well if you want. */ static void -_close_open_fds_safe(int start_fd, PyObject* py_fds_to_keep) +_setcloexec_open_fds_safe(int start_fd, PyObject* py_fds_to_keep) { int fd_dir_fd; fd_dir_fd = _Py_open(FD_DIR, O_RDONLY); if (fd_dir_fd == -1) { /* No way to get a list of open fds. */ - _close_fds_by_brute_force(start_fd, py_fds_to_keep); + _setcloexec_fds_by_brute_force(start_fd, py_fds_to_keep); return; } else { char buffer[sizeof(struct linux_dirent64)]; @@ -274,7 +287,7 @@ continue; /* Not a number. */ if (fd != fd_dir_fd && fd >= start_fd && !_is_fd_in_sorted_fd_sequence(fd, py_fds_to_keep)) { - while (close(fd) < 0 && errno == EINTR); + setcloexec(fd); } } } @@ -282,13 +295,14 @@ } } -#define _close_open_fds _close_open_fds_safe +#define _setcloexec_open_fds _setcloexec_open_fds_safe #else /* NOT (defined(__linux__) && defined(HAVE_SYS_SYSCALL_H)) */ -/* Close all open file descriptors from start_fd and higher. - * Do not close any in the sorted py_fds_to_keep list. +/* Set close to close on exec all open file descriptors from start_fd + * and higher. Do not set to close any in the sorted py_fds_to_keep + * list. * * This function violates the strict use of async signal safe functions. :( * It calls opendir(), readdir() and closedir(). Of these, the one most @@ -301,7 +315,7 @@ * http://womble.decadent.org.uk/readdir_r-advisory.html */ static void -_close_open_fds_maybe_unsafe(long start_fd, PyObject* py_fds_to_keep) +_setcloexec_open_fds_maybe_unsafe(long start_fd, PyObject* py_fds_to_keep) { DIR *proc_fd_dir; #ifndef HAVE_DIRFD @@ -324,7 +338,7 @@ proc_fd_dir = opendir(FD_DIR); if (!proc_fd_dir) { /* No way to get a list of open fds. */ - _close_fds_by_brute_force(start_fd, py_fds_to_keep); + _setcloexec_fds_by_brute_force(start_fd, py_fds_to_keep); } else { struct dirent *dir_entry; #ifdef HAVE_DIRFD @@ -339,19 +353,26 @@ continue; /* Not a number. */ if (fd != fd_used_by_opendir && fd >= start_fd && !_is_fd_in_sorted_fd_sequence(fd, py_fds_to_keep)) { - while (close(fd) < 0 && errno == EINTR); + setcloexec(fd); } errno = 0; } if (errno) { /* readdir error, revert behavior. Highly Unlikely. */ - _close_fds_by_brute_force(start_fd, py_fds_to_keep); + _setcloexec_fds_by_brute_force(start_fd, py_fds_to_keep); } closedir(proc_fd_dir); } } -#define _close_open_fds _close_open_fds_maybe_unsafe +/* + * You might think this should be _close_open_fds instead but closing + * all file descriptors not in a list can be racy when concurrently + * iterating over a system's list of open file descriptors and closing + * files at the same time so instead we mark the files to be closed + * later by the system. + */ +#define _setcloexec_open_fds _setcloexec_open_fds_maybe_unsafe #endif /* else NOT (defined(__linux__) && defined(HAVE_SYS_SYSCALL_H)) */ @@ -471,7 +492,7 @@ /* close FDs after executing preexec_fn, which might open FDs */ if (close_fds) { /* TODO HP-UX could use pstat_getproc() if anyone cares about it. */ - _close_open_fds(3, py_fds_to_keep); + _setcloexec_open_fds(3, py_fds_to_keep); } /* This loop matches the Lib/os.py _execvpe()'s PATH search when */ diff -r 110b95446ec2 Python/fileutils.c --- a/Python/fileutils.c Mon Jun 02 00:31:44 2014 +0100 +++ b/Python/fileutils.c Thu Jun 12 12:54:32 2014 -0700 @@ -1,4 +1,5 @@ #include "Python.h" +#include "setcloexec.h" #include "osdefs.h" #include @@ -622,11 +623,7 @@ #ifdef MS_WINDOWS HANDLE handle; DWORD flags; -#elif defined(HAVE_SYS_IOCTL_H) && defined(FIOCLEX) && defined(FIONCLEX) - int request; - int err; -#elif defined(HAVE_FCNTL_H) - int flags; +#else int res; #endif @@ -671,32 +668,8 @@ } return 0; -#elif defined(HAVE_SYS_IOCTL_H) && defined(FIOCLEX) && defined(FIONCLEX) - if (inheritable) - request = FIONCLEX; - else - request = FIOCLEX; - err = ioctl(fd, request, NULL); - if (err) { - if (raise) - PyErr_SetFromErrno(PyExc_OSError); - return -1; - } - return 0; - #else - flags = fcntl(fd, F_GETFD); - if (flags < 0) { - if (raise) - PyErr_SetFromErrno(PyExc_OSError); - return -1; - } - - if (inheritable) - flags &= ~FD_CLOEXEC; - else - flags |= FD_CLOEXEC; - res = fcntl(fd, F_SETFD, flags); + res = _Py_setcloexec(fd, inheritable); if (res < 0) { if (raise) PyErr_SetFromErrno(PyExc_OSError);