Charles-François Natali added the comment:
> Using self.assertLessEqual() would provide better message on error.
Done.
> You don't want to fix Python 2.7 and 3.3? It is a bug in my opinion.
Patch for 2.7 attached (without test though, because 2.7 lacks a
fd_status.py helper, which would make this quite complicated).
----------
Added file: http://bugs.python.org/file31424/subprocess_close-default.diff
Added file: http://bugs.python.org/file31425/subprocess_close-27.diff
_______________________________________
Python tracker <rep...@bugs.python.org>
<http://bugs.python.org/issue18763>
_______________________________________
diff -r 49e23a3adf26 Lib/test/test_subprocess.py
--- a/Lib/test/test_subprocess.py Wed Aug 21 13:26:34 2013 +0200
+++ b/Lib/test/test_subprocess.py Thu Aug 22 21:19:52 2013 +0200
@@ -1972,6 +1972,19 @@
self.assertRaises(OSError, os.waitpid, pid, 0)
self.assertNotIn(ident, [id(o) for o in subprocess._active])
+ def test_close_fds_after_preexec(self):
+ fd_status = support.findfile("fd_status.py", subdir="subprocessdata")
+
+ # FDs created by preexec_fn should be closed in the child
+ p = subprocess.Popen([sys.executable, fd_status],
+ stdout=subprocess.PIPE, close_fds=True,
+ preexec_fn=os.pipe)
+ output, ignored = p.communicate()
+
+ remaining_fds = set(map(int, output.split(b',')))
+
+ self.assertLessEqual(remaining_fds, set([0, 1, 2]))
+
@unittest.skipUnless(mswindows, "Windows specific tests")
class Win32ProcessTestCase(BaseTestCase):
diff -r 49e23a3adf26 Modules/_posixsubprocess.c
--- a/Modules/_posixsubprocess.c Wed Aug 21 13:26:34 2013 +0200
+++ b/Modules/_posixsubprocess.c Thu Aug 22 21:19:52 2013 +0200
@@ -412,17 +412,6 @@
POSIX_CALL(close(errwrite));
}
- if (close_fds) {
- int local_max_fd = max_fd;
-#if defined(__NetBSD__)
- local_max_fd = fcntl(0, F_MAXFD);
- if (local_max_fd < 0)
- local_max_fd = max_fd;
-#endif
- /* TODO HP-UX could use pstat_getproc() if anyone cares about it. */
- _close_open_fd_range(3, local_max_fd, py_fds_to_keep);
- }
-
if (cwd)
POSIX_CALL(chdir(cwd));
@@ -451,6 +440,18 @@
/* Py_DECREF(result); - We're about to exec so why bother? */
}
+ /* close FDs after executing preexec_fn, which might open FDs */
+ if (close_fds) {
+ int local_max_fd = max_fd;
+#if defined(__NetBSD__)
+ local_max_fd = fcntl(0, F_MAXFD);
+ if (local_max_fd < 0)
+ local_max_fd = max_fd;
+#endif
+ /* TODO HP-UX could use pstat_getproc() if anyone cares about it. */
+ _close_open_fd_range(3, local_max_fd, py_fds_to_keep);
+ }
+
/* This loop matches the Lib/os.py _execvpe()'s PATH search when */
/* given the executable_list generated by Lib/subprocess.py. */
saved_errno = 0;
diff -r 84d74eb7a341 Lib/subprocess.py
--- a/Lib/subprocess.py Wed Aug 21 18:25:00 2013 +0200
+++ b/Lib/subprocess.py Thu Aug 22 23:20:40 2013 +0200
@@ -1247,16 +1247,17 @@
os.close(fd)
closed.add(fd)
- # Close all other fds, if asked for
- if close_fds:
- self._close_fds(but=errpipe_write)
-
if cwd is not None:
os.chdir(cwd)
if preexec_fn:
preexec_fn()
+ # Close all other fds, if asked for - after
+ # preexec_fn(), which might open FDs
+ if close_fds:
+ self._close_fds(but=errpipe_write)
+
if env is None:
os.execvp(executable, args)
else:
_______________________________________________
Python-bugs-list mailing list
Unsubscribe:
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
