classification
Title: Popen.wait does not account for negative return codes
Type: behavior Stage: resolved
Components: Library (Lib) Versions:
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: MrTroble, eryksun, paul.moore, steve.dower, tim.golden, zach.ware
Priority: normal Keywords: patch

Created on 2020-07-24 15:54 by MrTroble, last changed 2020-07-27 14:40 by eryksun. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 21607 closed MrTroble, 2020-07-24 15:58
Messages (7)
msg374194 - (view) Author: Nico (MrTroble) * Date: 2020-07-24 15:54
Following problem occurred.

A C style program can have negative return codes. However this is not correctly implemented in the Windows API. Therefore it is being returned as unsigned long by the API hence it leads to ambiguity while comparing return codes.

For reference regarding this topic see:
https://docs.microsoft.com/en-us/cpp/cpp/main-function-command-line-args?redirectedfrom=MSDN&view=vs-2019
https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-getexitcodeprocess

I suggest a
msg374195 - (view) Author: Nico (MrTroble) * Date: 2020-07-24 15:58
I suggest implementing a singed conversation into _wait
msg374209 - (view) Author: Nico (MrTroble) * Date: 2020-07-24 19:55
I don't know if it should be a good idea to add a flag to turn off the negative conversation?
msg374212 - (view) Author: Nico (MrTroble) * Date: 2020-07-24 20:11
We apparently need a flag to ensure compatibility with the windows return codes

======================================================================
FAIL: test_disable_windows_exc_handler (test.test_faulthandler.FaultHandlerTests)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "D:\a\cpython\cpython\lib\test\test_faulthandler.py", line 825, in test_disable_windows_exc_handler
    self.assertEqual(exitcode, 0xC0000005)
AssertionError: -1073741819 != 3221225477
msg374219 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2020-07-25 00:46
In the Windows API, errors and exit status codes are all unsigned 32-bit integers. See SetLastError, GetLastError, ExitThread, ExitProcess, GetExitCodeThread, and GetExitCodeProcess. Even signed NTSTATUS and HRESULT values are commonly displayed as non-negative values, and, as exit status values, they're usually matched as unsigned integers. 

I think, if anything, it should suffice to document the range of `Popen.returncode` in Windows as a 32-bit unsigned integer.

---

Even POSIX systems in many cases have no use for the signed exit status from C exit(). The POSIX wait() and waitpid() system calls only provide the lower byte (i.e. masked by 0xFF) of the exit status. The subprocess module takes advantage of this on POSIX systems in order to reserve negative return codes to indicate termination by a signal. 

The newer waitid() system call should be able to return the signed 32-bit exit status. I think that's implemented on BSD and macOS, but waitid() in Linux 5.4 appears to only return the masked 8-bit status. For example:

    >>> p = subprocess.Popen(['python', '-c', 'import os; os._exit(-1)'])
    >>> os.waitid(os.P_PID, p.pid, os.WEXITED).si_status
    255
msg374248 - (view) Author: Nico (MrTroble) * Date: 2020-07-25 08:58
Yeah I see the point.

To me it does make no sense because even in the windows API they seam to not know whether to use a ulong or a uint nor does this reflect the actual depiction within the program. 

However it is probably not worth implementing a flag for that nor to change the behavior entirely. This will create more problems than it would fix ambiguity. 

Thanks for taking the time and sorry for the inconvenience.
msg374383 - (view) Author: Eryk Sun (eryksun) * (Python triager) Date: 2020-07-27 14:40
> in the windows API they seam to not know whether to use a ulong or a uint

This usage requires C `long int` to be the same size as `int` in 64-bit Windows.
History
Date User Action Args
2020-07-27 14:40:12eryksunsetresolution: not a bug
messages: + msg374383
2020-07-25 08:58:20MrTroblesetstatus: open -> closed

messages: + msg374248
stage: patch review -> resolved
2020-07-25 00:46:58eryksunsetnosy: + eryksun
messages: + msg374219
2020-07-24 20:11:31MrTroblesetmessages: + msg374212
2020-07-24 19:55:35MrTroblesetmessages: + msg374209
2020-07-24 19:30:43MrTroblesetversions: - Python 3.8
2020-07-24 17:23:47MrTroblesetcomponents: + Library (Lib), - Windows
2020-07-24 15:58:07MrTroblesetkeywords: + patch

stage: patch review
messages: + msg374195
pull_requests: + pull_request20749
2020-07-24 15:54:41MrTroblecreate