Windows: subprocess debug assertion on failure to execute the process

[segevfiner]

BPO	30121
Nosy	@terryjreedy, @pfmoore, @vstinner, @tjguk, @zware, @eryksun, @zooba, @segevfiner, @miss-islington
PRs	bpo-30121: Fix debug assert in subprocess on Windows #1224 bpo-30121: Write an unit test #3133 [3.6] bpo-30121: Fix debug assert in subprocess on Windows (#1224) #3173 bpo-30121: Fix test_subprocess for Windows Debug builds #5758 [3.7] bpo-30121: Fix test_subprocess for Windows Debug builds (GH-5758) #5759 [3.6] bpo-30121: Fix test_subprocess for Windows Debug builds (GH-5758) #5760

^{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:

assignee = None
closed_at = <Date 2018-01-09.21:35:27.662>
created_at = <Date 2017-04-20.22:43:55.579>
labels = ['3.7', 'library', 'OS-windows', 'type-crash']
title = 'Windows: subprocess debug assertion on failure to execute the process'
updated_at = <Date 2018-02-19.21:00:24.285>
user = 'https://github.com/segevfiner'

bugs.python.org fields:

activity = <Date 2018-02-19.21:00:24.285>
actor = 'miss-islington'
assignee = 'none'
closed = True
closed_date = <Date 2018-01-09.21:35:27.662>
closer = 'Segev Finer'
components = ['Library (Lib)', 'Windows']
creation = <Date 2017-04-20.22:43:55.579>
creator = 'Segev Finer'
dependencies = []
files = []
hgrepos = []
issue_num = 30121
keywords = []
message_count = 12.0
messages = ['292002', '300458', '300460', '300495', '300654', '300655', '300658', '308532', '309741', '312365', '312371', '312373']
nosy_count = 9.0
nosy_names = ['terry.reedy', 'paul.moore', 'vstinner', 'tim.golden', 'zach.ware', 'eryksun', 'steve.dower', 'Segev Finer', 'miss-islington']
pr_nums = ['1224', '3133', '3173', '5758', '5759', '5760']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue30121'
versions = ['Python 3.6', 'Python 3.7']

[segevfiner]

subprocess triggers a debug assertion in the CRT on failure to execute the process due to closing the pipe *handles* in the except clause using os.close rather than .Close() (os.close closes CRT file descriptors and not handles).

In addition to that once this is fixed there is also a double free/close since we need to set self._closed_child_pipe_fds = True once we closed the handles in _execute_child so they won't also be closed in init.

To reproduce, do this in a debug build of Python:

    import subprocess
    subprocess.Popen('exe_that_doesnt_exist.exe', stdout=subprocess.PIPE)

See: #1218 (comment)

[terryjreedy]

I determined in bpo-31228 (also on Windows, closed as a duplicate) that a debug build, bad file, and subprocess.PIPE are all required. Has this been tried on non-Windows? I confirmed the crash on 3.6. I do not have a 2.7 repository build.

The two lines above constitute a unittest that currently fails on a Windows debug build. Even if there is no such buildbot, I think a test should be added that will fail on developer machines with such builds. It should be skipped on non-debug and perhaps non-Windows machines.

[vstinner]

See #1224 and http://bugs.python.org/issue30121

[vstinner]

New changeset 4d38517 by Victor Stinner (Segev Finer) in branch 'master':
bpo-30121: Fix debug assert in subprocess on Windows (bpo-1224)
4d38517

[vstinner]

New changeset 9a83f65 by Victor Stinner in branch 'master':
Add test_subprocess.test_nonexisting_with_pipes() (bpo-3133)
9a83f65

[vstinner]

New changeset e76cb43 by Victor Stinner in branch '3.6':
[3.6] bpo-30121: Fix debug assert in subprocess on Windows (bpo-1224) (bpo-3173)
e76cb43

[vstinner]

Python 2.7 doesn't seem to be affected by this issue. Extract of Popen.__init__:

    try:
        self._execute_child(args, executable, preexec_fn, close_fds,
                            cwd, env, universal_newlines,
                            startupinfo, creationflags, shell, to_close,
                            p2cread, p2cwrite,
                            c2pread, c2pwrite,
                            errread, errwrite)
    except Exception:
        # Preserve original exception in case os.close raises.
        exc_type, exc_value, exc_trace = sys.exc_info()

            for fd in to_close:
                try:
                    if mswindows:
                        fd.Close()
                    else:
                        os.close(fd)
                except EnvironmentError:
                    pass

On Windows, fd.Close() is always used.

[vstinner]

Oops, I merged the pull requests, but I forgot to close the issue.