[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Roundup Robot added the comment: New changeset 2bdd984a55ac by Gregory P. Smith in branch '2.7': Fix issue #16140 bug that the fix to issue #16327 added - don't double http://hg.python.org/cpython/rev/2bdd984a55ac -- ___ Python tracker

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Changes by Gregory P. Smith : -- resolution: -> fixed stage: patch review -> committed/rejected status: open -> closed ___ Python tracker ___ ___

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Roundup Robot added the comment: New changeset e67620048d2f by Gregory P. Smith in branch '2.7': Fixes issue #16327: The subprocess module no longer leaks file descriptors http://hg.python.org/cpython/rev/e67620048d2f -- ___ Python tracker

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Roundup Robot added the comment: New changeset 63ff4c9a2ed2 by Gregory P. Smith in branch '3.2': Fixes issue #16327: The subprocess module no longer leaks file descriptors http://hg.python.org/cpython/rev/63ff4c9a2ed2 New changeset a6a6c349af7e by Gregory P. Smith in branch '3.3': Fixes issue #1

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Gregory P. Smith added the comment: Thanks! I'm looking into applying these tonight (including 3.2) with a couple minor edits. -- ___ Python tracker ___

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Mark Gius added the comment: My contributor form has been accepted. Anything else I should be doing to work towards getting a fix applied? -- ___ Python tracker ___ ___

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Mark Gius added the comment: I've also submitted the contributor form requested. -- ___ Python tracker ___ ___ Python-bugs-list mailin

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Mark Gius added the comment: Here's more or less the same fix and test on 2.7. I jumped through the hoop to preserve the original exception and traceback even if os.close() raises an exception. This follows the 3.3 branch's cleanup behavior of silently suppressing errors in the cleanup code.

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Mark Gius added the comment: Patch fixes and tests fd leak on Python 3.3. Test fails without fix, passes with fix. I found an existing test looking for fd leaks for another bug. Borrowed the verification bits from it. There were some other test failures when I ran the subprocess suite on my

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Gregory P. Smith added the comment: Stubbing _execute_child out for a test is easiest. No need to craft ways to cause an actual fork failure. -- ___ Python tracker ___ __

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Mark Gius added the comment: Doesn't exhibit when execve fails, because by the time execve has been reached we've closed the pipes that we're supposed to close on the parent process, and the pipes that are meant to remain open on the parent process get caught by existing cleanup code. It's un

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Charles-François Natali added the comment: > The problem with using RLIMIT is that the testsuite could be executing > several tests in parallel using independent threads, for instance. You don't > want to influence unrelated tests. That's why I suggested to run it in a subprocess: this is used

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Jesús Cea Avión added the comment: The problem with using RLIMIT is that the testsuite could be executing several tests in parallel using independent threads, for instance. You don't want to influence unrelated tests. Overiding private methods is ugly, but if the code evolves the test would b

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Charles-François Natali added the comment: Also, I didn't check, but if the problems also occurs on execve() failure, then it's much simpler: simply call Popen() with an invalid/nonexisting executable. -- ___ Python tracker

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Charles-François Natali added the comment: > No automated testing included because I'm not entirely sure how to replicate > this without eating up a ton of ram or doing something naughty with ulimit. Simply use RLIMIT_NPROC, from a subprocess: """ $ cat /tmp/test.py import subprocess ps = [ ]

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Jesús Cea Avión added the comment: Replace "except BaseException as e :" with just "except:". It was a remnant of my tests. -- ___ Python tracker ___ ___

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Jesús Cea Avión added the comment: Python2 management should be something like: """ Python 2.7.3 (default, Apr 12 2012, 13:11:53) [GCC 4.4.3] on linux2 Type "help", "copyright", "credits" or "license" for more information. >>> try : ... 1/0 ... except BaseException as e : ... try : ...

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Jesús Cea Avión added the comment: http://stackoverflow.com/questions/8997431/is-there-any-way-to-access-nested-or-re-raised-exceptions-in-python """ This is known as Exception Chaining and is suported in Python 3. PEP 3134: http://www.python.org/dev/peps/pep-3134/ In Python 2, the old excepti

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Jesús Cea Avión added the comment: In fact, nested exception management in python 2 and python 3 actually diverges. BEWARE: (Python 3 does the right thing, once again :-) """ Python 2.7.3 (default, Apr 12 2012, 13:11:53) [GCC 4.4.3] on linux2 Type "help", "copyright", "credits" or "license" fo

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Jesús Cea Avión added the comment: Mark, could you consider to fill&send a contributor form agreement? http://www.python.org/psf/contrib/ -- ___ Python tracker ___ _

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Jesús Cea Avión added the comment: The cleanup code in python 3 validates my idea of simplifying cleanup moving "_execute_child()" after the platform specific code. I wonder what "raise" will actually raise if this cleanup code catches & ignores "close()" exception :-). -- __

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Mark Gius added the comment: attachment against 2.7 if I understand this code correctly, the fix shouldn't need to be fixed separately on windows and Linux, since the thing handled by init is just a file descriptor. Good idea on the testing. I'll give that a shot tomorrow. I think 3.3 will n

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Gregory P. Smith added the comment: python 3 already catches all exceptions and handles closing of p2cwrite, c2pread and errread here. i don't know which branch this patch is against. Regardless, it makes sense that the other fd's, if created by us, also need to be cleaned up. The code also

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Jesús Cea Avión added the comment: I would catch ALL exceptions, not only "OSError". An easy way to test this would be to test a subclass of Popen with "_execute_child()" method overrided for always raising an exception. On Unix the test could just open six fds, close them taking note of the v

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Changes by Gregory P. Smith : -- assignee: -> gregory.p.smith ___ Python tracker ___ ___ Python-bugs-list mailing list Unsubscribe:

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Mark Gius added the comment: Patch now only closes pipe fds created by Popen -- Added file: http://bugs.python.org/file27720/fd_leak_fix_v2.diff ___ Python tracker ___ __

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Mark Gius added the comment: Just read the docs for stdin and stdout. This patch will indtroduce a bug where a provided file descriptor can be closed. This definitely shouldn't close a file descriptor that isn't created by _get_handles(). I'll update. -- ___

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

Changes by Antoine Pitrou : -- nosy: +gregory.p.smith, neologix stage: -> patch review versions: +Python 3.2, Python 3.3, Python 3.4 ___ Python tracker ___ _

[issue16327] subprocess.Popen leaks file descriptors on os.fork() failure

New submission from Mark Gius: When subprocess.Popen is called with subprocess.PIPE and os.fork() fails (due to insufficient memory for example), the pipes created by _get_handles() are not closed. Steps to reproduce: 1) Launch a python shell, import subprocess 2) Create a situation where os.