bpo-30121: Fix debug assert in subprocess on Windows#1224
bpo-30121: Fix debug assert in subprocess on Windows#1224vstinner merged 2 commits intopython:masterfrom
Conversation
|
@segevfiner, thanks for your PR! By analyzing the history of the files in this pull request, we identified @gpshead, @tim-one and @rosslagerwall to be potential reviewers. |
|
ping @Haypo since you marked yourself for review some time ago but might have forgotten about this 😉 |
vstinner
left a comment
vstinner
left a comment
There was a problem hiding this comment.
Would it be possible to write an unit test? Try to mock the _execute_child() method to raise an exception for example?
This is caused by closing HANDLEs using os.close which is for CRT file descriptors and not for HANDLEs.
|
@Haypo Plenty of tests already trigger this. And any simple test that just tries to run a non-existent executable with io redirection will also trigger this. But libregrtest does I did encounter one test in I don't know how to write a test that checks specifically that there isn't a debug assertion under |
segevfiner
force-pushed
the
issue-30121-windows-subprocess-debug-assert
branch
from
72ed71a to
6dd5a06
Compare
|
Sorry, it took me a while to understand the fixed bug, but now I understood the 3 changes and they all LGTM. I had to play with subprocess to understand why we need all these changes. |
|
I wrote an unit test for this change: PR #3133. |
…3173) * bpo-30121: Fix debug assert in subprocess on Windows (#1224) * bpo-30121: Fix debug assert in subprocess on Windows This is caused by closing HANDLEs using os.close which is for CRT file descriptors and not for HANDLEs. * bpo-30121: Suppress debug assertion in test_subprocess when ran directly (cherry picked from commit 4d38517) * Add test_subprocess.test_nonexisting_with_pipes() (#3133) bpo-30121: Test the Popen failure when Popen was created with pipes. Create also NONEXISTING_CMD variable in test_subprocess.py. (cherry picked from commit 9a83f65)
4 participants
This is caused by closing HANDLEs using os.close which is for CRT file
descriptors and not for HANDLEs.
http://bugs.python.org/issue30121