New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
subprocess.Popen does not release process handles if process cannot be started #47460
Comments
Run the following code on Windows: import subprocess, os
file = open("filename", "w")
try:
proc = subprocess.Popen("nosuchprogram", stdout=file)
except OSError:
file.close()
os.remove("filename") This produces the following exception: Traceback (most recent call last):
File "C:\processown.py", line 10, in <module>
os.remove("filename")
WindowsError: [Error 32] The process cannot access the file because it
is being used by another process: 'filename' When the CreateProcess call fails the subprocess module should release As Tim Golden points out, this can be worked around by doing |
The attached file sp-3.py simulates what I think is happening within the There is no hope of closing the file handle since it was local to the On the surface, the obvious fix would be a try/except block around the |
A note on workarounds, the garbage collector seems to release the |
I tried closing -all- of the handles listed here (the std ones used as
someone with more windows willingness/knowledge/fu should take this one on. |
Hello. I had lecture about exception & frames on bpo-3515. When Lib/subprocess.py (814)'s CreateProcess() raises exception, import subprocess, os
file = open("filename", "w")
try:
proc = subprocess.Popen("nosuchprogram", stdout=file)
except OSError:
pass try: file.close()
os.remove("filename") # runs fine So, I think subprocess module's practice which relys on refcount GC |
This is not limited to Windows. I experience this bug on Mac OS X and http://mail.python.org/pipermail/python-list/2008-August/505753.html Attachment 3210.py is a reduced testcase. It attempts to execute nocmd, For quick reproduction, set a low but reasonable limit on the number of mark@anodizer bash$ ulimit -n 256
mark@anodizer bash$ python 3210.py
250
Traceback (most recent call last):
File "3210.py", line 11, in <module>
raise e
OSError: [Errno 24] Too many open files |
Is this a duplicate of this already fixed issue: bpo-5179 ? |
No, this is not duplicate of bpo-5179. That issue described handle was import subprocess, os, sys
file = open("filename", "w")
try:
proc = subprocess.Popen("nosuchprogram", stdout=file)
except OSError:
file.close()
sys.exc_clear() # here we go....
os.remove("filename") # now we can delete file! subprocess is implemented using sp_handle_type in PC/_subprocess.c |
Probably we can fix this issue by calling Close() of sp_handle_type |
Same problem in 3.1.2 |
Patch attached with code & test which fixes this within _subprocess.c at least for Windows |
Patch added for 31 branch |
Patch added for 27 branch |
Tim, |
Brian, I'm not sure that the test as rewritten will exercise the error. The key is that the traceback object will prevent the handles In other words, I'm not testing that an exception is raised (which |
Ah ok. I got hooked onto new unittest stuff and overdid it. Whoops. In that case, I guess just the last lines converting your "assert_" to "assertFalse" would be my only suggestion. |
Committed in r83759 r83760 r83761 |
Sorry for posting to closed entry, but I think handle should be closed in Lib/subprocess.py not in PC/_subprocess.c. I noticed following code showed strange error. import subprocess
for _ in xrange(2):
stdout = open("stdout.txt", "w")
try:
p = subprocess.Popen(["unknown"], stdout=stdout)
except WindowsError:
pass // error close failed in file object destructor: |
Blast. Thanks; I'll have to rework those patches then. |
OK, the issue identified by Hirokazu Yamamoto in msg113106 only actually affects the 2.x series, because of the awkwardly multiple-level interaction between file handles. The io rewrite in 3.x seems not to suffer the same way. Nonetheless, the proposed adhok.patch (slightly updated to match the current codebase) seems to cover all the cases, and is rather more elegant. So I'll revert the C module changes and apply that instead to all branches. |
Committed in r83815, r83816, r83817 |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: