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
the handles it provides. Unfortunately it seems to raise WindowsError
before doing this.

See also
http://groups.google.com/group/comp.lang.python/browse_thread/thread/6157691ea3324779/6274e9f8bc8a71ee?hl=en#6274e9f8bc8a71ee

As Tim Golden points out, this can be worked around by doing
os.close(file.fileno()) at the end instead of file.close()

The attached file sp-3.py simulates what I think is happening within the
subprocess module. Note that the OS handle is duplicated to allow
inheritance and then left unclosed on failure. If it is explicitly
closed, the file can be removed.

There is no hope of closing the file handle since it was local to the
__init__ method which failed but was not closed before exit and is now
inaccessible.

On the surface, the obvious fix would be a try/except block around the
CreateProcess call (or its caller) which would then release whatever
handles were needed. I'll try to get the time to put a patch together,
but it would be useful to have confirmation of my theory.

A note on workarounds, the garbage collector seems to release the
handles when the function exits, so removing the file in a caller works
for me. However Tim's proposed fix with os.close didn't do so.

I tried closing -all- of the handles listed here (the std ones used as
input to _get_handles and the pipe read/write ones returned) on an
OSError during CreateProcess but the problem still occurs for me using
the test code in msg68787.

        (p2cread, p2cwrite,
         c2pread, c2pwrite,
         errread, errwrite) = self._get_handles(stdin, stdout, stderr)

someone with more windows willingness/knowledge/fu should take this one on.

Hello. I had lecture about exception & frames on issue3515.

When Lib/subprocess.py (814)'s CreateProcess() raises exception,
p2cread will be freed by refcount GC, but it never happen before
os.remove() because sys.exc_traceback holds reference to frame
which has p2cread.

import subprocess, os

file = open("filename", "w")
try:
    proc = subprocess.Popen("nosuchprogram", stdout=file)
except OSError:
    pass

try:
    raise RuntimeError() # hack to clear previous exc_traceback
except:
    pass

file.close()
os.remove("filename") # runs fine

So, I think subprocess module's practice which relys on refcount GC
is not good. (p2cread is PC/_subprocess.c 's sp_handle_object, so
automatically freed by refcount GC) I don't think attached "adhok.patch"
is enough, but seems to fix this issue at least.

This is not limited to Windows.  I experience this bug on Mac OS X and
Linux as well.

http://mail.python.org/pipermail/python-list/2008-August/505753.html

Attachment 3210.py is a reduced testcase.  It attempts to execute nocmd,
which should not exist.  The expected behavior is for OSError to be
thrown each time, with ENOENT or EACCES set in the errno field,
depending on the environment.  Due to this bug, Python will hit the file
descriptor limit at some point instead.

For quick reproduction, set a low but reasonable limit on the number of
maximum open files:

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: issue5179 ?

No, this is not duplicate of issue5179. That issue described handle was
leaked when exception occurred. But this issue is not *leak*. See
following code.

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
(windows). This object is in exception stack frame(?), so handle lives
until another exception occurs or explicitly sys.exc_clear() is called.

Probably we can fix this issue by calling Close() of sp_handle_type
somewhere in Lib/subprocess.py, but I have no patch now.

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,
I updated your test to use some of the newer and preferred unittest features and made a change to do the common stuff in loops. The _subprocess.c changes look fine to me. See attached issue3210_py3k.diff.

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
from being finalised until it is itself finalised. After your
change I expect the handles to release anyway since the traceback
is already finalised.

In other words, I'm not testing that an exception is raised (which
is what the with assertRaises would suggest); I'm testing that
*within the lifetime of the exception* it's still possible to
remove the files which were passed as stdin/out/err where it wasn't
previously.

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:
IOError: [Errno 9] Bad file descriptor

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