classification
Title: Windows: subprocess debug assertion on failure to execute the process
Type: crash Stage: resolved
Components: Library (Lib), Windows Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Segev Finer, eryksun, miss-islington, paul.moore, steve.dower, terry.reedy, tim.golden, vstinner, zach.ware
Priority: normal Keywords:

Created on 2017-04-20 22:43 by Segev Finer, last changed 2018-02-19 21:00 by miss-islington. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 1224 merged Segev Finer, 2017-04-20 23:22
PR 3133 merged vstinner, 2017-08-18 13:27
PR 3173 merged vstinner, 2017-08-21 21:53
PR 5758 merged zach.ware, 2018-02-19 19:49
PR 5759 merged miss-islington, 2018-02-19 20:02
PR 5760 merged miss-islington, 2018-02-19 20:04
Messages (12)
msg292002 - (view) Author: Segev Finer (Segev Finer) * Date: 2017-04-20 22:43
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: https://github.com/python/cpython/pull/1218#discussion_r112550959
msg300458 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2017-08-17 21:18
I determined in #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.
msg300460 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-08-17 22:13
See https://github.com/python/cpython/pull/1224 and http://bugs.python.org/issue30121
msg300495 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-08-18 13:18
New changeset 4d3851727fb82195e4995c6064b0b2d6cbc031c4 by Victor Stinner (Segev Finer) in branch 'master':
bpo-30121: Fix debug assert in subprocess on Windows (#1224)
https://github.com/python/cpython/commit/4d3851727fb82195e4995c6064b0b2d6cbc031c4
msg300654 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-08-21 21:51
New changeset 9a83f651f31b47b3f6c8b210f7807b26e8c373a5 by Victor Stinner in branch 'master':
Add test_subprocess.test_nonexisting_with_pipes() (#3133)
https://github.com/python/cpython/commit/9a83f651f31b47b3f6c8b210f7807b26e8c373a5
msg300655 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-08-21 22:15
New changeset e76cb435563cd14bb085942dfefbb469b8f40aa9 by Victor Stinner in branch '3.6':
[3.6] bpo-30121: Fix debug assert in subprocess on Windows (#1224) (#3173)
https://github.com/python/cpython/commit/e76cb435563cd14bb085942dfefbb469b8f40aa9
msg300658 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-08-21 22:24
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.
msg308532 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2017-12-18 09:41
Oops, I merged the pull requests, but I forgot to close the issue.
msg309741 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-01-09 22:11
> Oops, I merged the pull requests, but I forgot to close the issue.

Oops, I forgot to close the issue, again... Thanks Segev for closing it, finally :-)
msg312365 - (view) Author: Zachary Ware (zach.ware) * (Python committer) Date: 2018-02-19 20:02
New changeset 5537646bfacec463b450871dde31cb06c44a0556 by Zachary Ware in branch 'master':
bpo-30121: Fix test_subprocess for Windows Debug builds (GH-5758)
https://github.com/python/cpython/commit/5537646bfacec463b450871dde31cb06c44a0556
msg312371 - (view) Author: miss-islington (miss-islington) Date: 2018-02-19 20:49
New changeset ef0bb5c7b76a49a5f3c5b85b5f9112cfefe54328 by Miss Islington (bot) in branch '3.6':
bpo-30121: Fix test_subprocess for Windows Debug builds (GH-5758)
https://github.com/python/cpython/commit/ef0bb5c7b76a49a5f3c5b85b5f9112cfefe54328
msg312373 - (view) Author: miss-islington (miss-islington) Date: 2018-02-19 21:00
New changeset 622a824802771fc5aa133ae92101bc8303360294 by Miss Islington (bot) in branch '3.7':
bpo-30121: Fix test_subprocess for Windows Debug builds (GH-5758)
https://github.com/python/cpython/commit/622a824802771fc5aa133ae92101bc8303360294
History
Date User Action Args
2018-02-20 00:34:13steve.dowerlinkissue32764 superseder
2018-02-19 21:00:24miss-islingtonsetmessages: + msg312373
2018-02-19 20:49:49miss-islingtonsetnosy: + miss-islington
messages: + msg312371
2018-02-19 20:04:14miss-islingtonsetpull_requests: + pull_request5538
2018-02-19 20:02:59miss-islingtonsetpull_requests: + pull_request5537
2018-02-19 20:02:41zach.waresetmessages: + msg312365
2018-02-19 19:49:35zach.waresetpull_requests: + pull_request5536
2018-01-09 22:11:58vstinnersetmessages: + msg309741
2018-01-09 21:35:27Segev Finersetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2017-12-18 09:41:35vstinnersetmessages: + msg308532
2017-08-21 22:24:54vstinnersetmessages: + msg300658
2017-08-21 22:15:26vstinnersetmessages: + msg300655
2017-08-21 21:53:13vstinnersetpull_requests: + pull_request3209
2017-08-21 21:51:33vstinnersetmessages: + msg300654
2017-08-18 13:27:56vstinnersetpull_requests: + pull_request3167
2017-08-18 13:18:17vstinnersetmessages: + msg300495
2017-08-17 22:13:12vstinnersetmessages: + msg300460
2017-08-17 21:18:32terry.reedysetversions: + Python 3.6
nosy: + terry.reedy

messages: + msg300458

type: behavior -> crash
stage: patch review
2017-08-17 18:52:45eryksunlinkissue31228 superseder
2017-04-21 16:29:11vstinnersetnosy: + vstinner
2017-04-20 23:22:46Segev Finersetpull_requests: + pull_request1346
2017-04-20 22:43:55Segev Finercreate