os.wait() and os.waitpid() returns a "status" number which is not easy to return. It's made of two information: (how the process completed, value).

The usual way to handle it is to use a code which looks like:

    if os.WIFSIGNALED(status):
        self.returncode = -os.WTERMSIG(status)
    elif os.WIFEXITED(status):
        self.returncode = os.WEXITSTATUS(status)
    elif os.WIFSTOPPED(status):
        self.returncode = -os.WSTOPSIG(status)
    else:
        raise Exception("... put your favorite error message here ...")

It's not convenient to have to duplicate this code each time we have to handle a wait status.

Moreover, WIFSTOPPED() is commonly treated as "the process was killed by a signal", whereas the process is still alive but was only stopped. WIFSTOPPED() should only happen when the process is traced (by ptrace), or if waitpid() was called with WUNTRACED option.

The common case is not to trace a process or to use WUNTRACED. Moreover, if WIFSTOPPED() is true, the process is still alive and can continue its execution. It's bad to consider it as completed.


The subprocess module has such bug: Popen._handle_exitstatus() returns -os.WSTOPSIG(sts) if os.WIFSTOPPED(sts) is true.


On the other side, the pure Python implementation os._spawnvef() calls again waitpid() if WIFSTOPPED() is true. That sounds like a better behavior.

    while 1:
        wpid, sts = waitpid(pid, 0)
        if WIFSTOPPED(sts):
            continue

        elif WIFSIGNALED(sts):
            return -WTERMSIG(sts)
        elif WIFEXITED(sts):
            return WEXITSTATUS(sts)
        else:
            raise OSError("Not stopped, signaled or exited???")

But I'm not sure how WIFSTOPPED() can be true, since this function creates a child process using os.fork() and it doesn't use os.WUNTRACED flag.


I propose to add a private os._wait_status_to_returncode(status) helper function:
---
os._wait_status_to_returncode(status) -> int

Convert a wait() or waitpid() status to a returncode.

If WIFEXITED(status) is true, return WEXITSTATUS(status).
If WIFSIGNALED(status) is true, return -WTERMSIG(status).
Otherwise, raise a ValueError.

If the process is being traced or if waitpid() was called with WUNTRACED
option, the caller must first check if WIFSTOPPED(status) is true.
This function must not be called if WIFSTOPPED(status) is true.
---

I'm not sure if it's a good idea to add the helper as a private function. Someone may discover it and starts to use it. If we decide to make it public tomorrow, removing os._wait_status_to_returncode() would break code.

Maybe it's better to directly a public function? But I'm not sure if it's useful, nor if the function name is good, nor if good to helper an function function directly in the os module.

Maybe such helper should be added to shutil instead which is more the "high-level" flavor of the os module?

I chose to add it to the os module for different reasons:

* Existing code using os.WEXITSTATUS() and friends usually only uses the os module.
* It's convenient to be able to use os._wait_status_to_returncode(status) in the subprocess module without adding a dependency (import) on the shutil module.
* os.wait() and os.waitpid() live in the os module: it's convenient to have an helper functon in the same module.

What do you think?

* Is it worth it to add os._wait_status_to_returncode() helper function?

* If you like the idea, propose a better name!

* Should it remain private first?

Hum, I changed my mind and I think that it's worth it to make the function public. Moreover, I prefer "exitcode", since it is closer to "WEXITSTATUS" name than "returncode". So I renamed the function os.status_to_exitcode().

Advantages of the new new function compared to exiting code:

* the function guarantee that result >= 0 means that the process exited and result < 0 means that the process exited due to a signal (was killed by a signal)
* the function raises a well defined exception (ValueError) if the WIFSTOPPED() is true or if the status is unknown: it prevents to misuse WIFSTOPPED()

I modified my PR to add Windows support. On Windows, os.waitpid() status also requires an operation (shif right by 8 bits) to get an exitcode from the waitpid status. So IMO it's worth it to add it to Windows as well, which makes the function even more useful ;-)

Interesting information about process "exit status code":
https://en.wikipedia.org/wiki/Exit_status

The function can be used to convert the result of the following functions:

* os.system() (on Unix)
* os.wait()
* os.waitpid()
* os.wait3()
* os.wait4()
* pty.spawn()

Note: waitid() has a different API, si_status can be used directly, its meaning depends on si_code.

--

First, I proposed os._wait_status_to_returncode() name.

I renamed it to os.status_to_exitcode().

Maybe the "status" term used alone is too general. 

I'm not sure neither if it's correct to write "exitcode" a single word. For example, Windows uses GetExitCodeProcess() name: "Exit Code" means that they are two separated words. The Python os documentation uses "exit code" and "exit status", but never "exitcode" or "exitstatus".

The system() function manual page says:

"""
RETURN VALUE
  (...)
   In the last two cases, the return value is a "wait status" that can be examined using the macros described in waitpid(2).  (i.e., WIFEXITED(), WEXITSTATUS(), and so on).
"""

Python pty.spawn() documentation says: "(...) returns the status value from os.waitpid() on the child process".

--

In my current PR, the function documentation is: "Convert an exit status to an exit code"

Other name ideas:

* exit_status_to_code()
* exit_status_to_exit_code()
* wait_status_to_exit_code()

The Wikipedia article is called "Exit status" and then tells about "exit status code" :-) So it can be surprising to have a function supposed to convert an "exit status" into an "exit code".

It seems like "Convert a waitpid() wait status to an exit code" is the best documentation and so that wait_status_to_exit_code() is the most correct and least confusing name, even if it's longer than other name candidates.

> Other name ideas:
> * wait_status_to_exit_code()

Well, anothe option is:

* waitstatus_to_exitcode()

While the documentation uses "exit code", the code commonly uses "exitcode" or "returncode". Moreover, in the os module, underscore is not used to separated words in function names. Examples: "getenv" not "get_env", "setpriority" not "set_priority", etc.

Using "_to_" in the function name reduces the risk of conflict with a future addition to the libc. It's rare that libc function names use "_". One of the few exception: get_current_dir_name() which is a glibc extension.

Ok, I chose os.waitstatus_to_exitcode() name. I updated my PR.

> On Windows, os.waitpid() status also requires an operation (shif 
> right by 8 bits) to get an exitcode from the waitpid status. 

FWIW, I wouldn't recommend relying on os.waitpid to get the correct process exit status in Windows. Status codes are 32 bits and generally all bits are required. os.waitpid left shifts the exit status by 8 bits in a dubious attempt to return a result that's "more like the POSIX waitpid". In particular, a program may exit abnormally with an NTSTATUS [1] or HRESULT [2] code such as STATUS_DLL_NOT_FOUND (0xC000_0135) or STATUS_CONTROL_C_EXIT (0xC000_013A).

[1]: https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-erref/87fba13e-bf06-450e-83b1-9241dc81e781
[2]: https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-erref/0642cb2f-2075-4469-918c-4441e69c548a

Eryk:
> FWIW, I wouldn't recommend relying on os.waitpid to get the correct process exit status in Windows. Status codes are 32 bits and generally all bits are required. os.waitpid left shifts the exit status by 8 bits in a dubious attempt to return a result that's "more like the POSIX waitpid". In particular, a program may exit abnormally with an NTSTATUS [1] or HRESULT [2] code such as STATUS_DLL_NOT_FOUND (0xC000_0135) or STATUS_CONTROL_C_EXIT (0xC000_013A).

os.waitpid() calls _cwait() on Windows and uses "status << 8".

The result is a Python object. IMHO it's ok if the shifted result ("status") is larger than 32 bits. But I'm not sure that the current os.waitpid() implementation handles integer overflow correctly...

When I look at GetExitCodeProcess() documentation, I don't see any distinction between "normal exit" and a program terminated by TerminateProcess(). The only different is the actual exit code:
https://docs.microsoft.com/en-us/windows/win32/api/processthreadsapi/nf-processthreadsapi-getexitcodeprocess

Do you suggest that os.waitstatus_to_exitcode() result should be negative if a process was terminated by TerminateProcess()?

--

My PR 19201 is based on the current Python implementation and assumptions used in the current code. But I don't think that what you wrote can be an API issue. It's more the opposite, if tomorrow we want to encode the status of a terminated process differently, it will be easier if os.waitstatus_to_exitcode() is available, no?

New changeset 278c1e159c970da6cd6683d18c6211f5118674cc by Victor Stinner in branch 'master':
bpo-40094: Add test.support.wait_process() (GH-19254)
https://github.com/python/cpython/commit/278c1e159c970da6cd6683d18c6211f5118674cc

New changeset 27c6231f5827fe17c6cb6f097391931f30b511ec by Victor Stinner in branch 'master':
bpo-40094: Enhance fork and wait tests (GH-19259)
https://github.com/python/cpython/commit/27c6231f5827fe17c6cb6f097391931f30b511ec

New changeset a9f9687a7ce25e7c0c89f88f52db323104668ae0 by Victor Stinner in branch 'master':
bpo-40094: Enhance threading tests (GH-19260)
https://github.com/python/cpython/commit/a9f9687a7ce25e7c0c89f88f52db323104668ae0

New changeset 40bfdb1594189f3c0238e5d2098dc3abf114e200 by Victor Stinner in branch 'master':
bpo-40094: Add _bootsubprocess._waitstatus_to_exitcode (GH-19264)
https://github.com/python/cpython/commit/40bfdb1594189f3c0238e5d2098dc3abf114e200

New changeset 16d75675d2ad2454f6dfbf333c94e6237df36018 by Victor Stinner in branch 'master':
bpo-31160: Fix race condition in test_os.PtyTests (GH-19263)
https://github.com/python/cpython/commit/16d75675d2ad2454f6dfbf333c94e6237df36018

New changeset 6b982c22e5fdbfecc24e440515b63f7253f695c4 by Victor Stinner in branch 'master':
bpo-40094: Add run_command() to setup.py (GH-19266)
https://github.com/python/cpython/commit/6b982c22e5fdbfecc24e440515b63f7253f695c4

New changeset afeaea2d6e346f627b24cc9e84e2986a7266a70e by Victor Stinner in branch 'master':
bpo-40094: Add missing import to wait_process() (GH-19268)
https://github.com/python/cpython/commit/afeaea2d6e346f627b24cc9e84e2986a7266a70e

> It's more the opposite, if tomorrow we want to encode the status 
> of a terminated process differently, it will be easier if 
> os.waitstatus_to_exitcode() is available, no?

This new status-to-exitcode function applies to Windows waitpid() only due to a design choice in CPython -- not the operating system. The current waitpid() implementation assumes it's okay to discard the upper 8 bits of the exit status, which can lose important information. Maybe it's best to address Windows support in a new issue that also addresses the design of waitpid() in 3.9, in particular if this would change the design of the new function -- at the very least with regard to data type (e.g. `int status` doesn't have the required range).

Off topic: Despite the function name, waitpid in Windows takes a process handle, such as is returned by os.spawn*, and not a process ID, such as is required by os.kill. The current documentation is sometimes clear on this detail but sometimes confusingly mixes up "handle" and "id" in Windows-only sections.

> The result is a Python object. IMHO it's ok if the shifted result 
> ("status") is larger than 32 bits. But I'm not sure that the 
> current os.waitpid() implementation handles integer overflow 
> correctly...

The overflow problem could be addressed by using a 64-bit value for the status in os_waitpid_impl and elsewhere.

> Do you suggest that os.waitstatus_to_exitcode() result should be 
> negative if a process was terminated by TerminateProcess()?

Returning a signed result is an interesting suggestion. The native process exit status is actually an NTSTATUS value, and NTSTATUS and HRESULT codes are signed, with failure codes (i.e. errors and warnings) reported as negative values. That said, the exit status gets handled as an unsigned value in the Windows API, e.g. ExitProcess, TerminateProcess, and GetExitCodeProcess.

> When I look at GetExitCodeProcess() documentation, I don't see any 
> distinction between "normal exit" and a program terminated by 
> TerminateProcess(). The only different is the actual exit code:

In almost all cases a process terminates via TerminateProcess -- or rather the internal NTAPI function NtTerminateProcess. For a clean exit via ExitProcess (i.e. native RtlExitUserProcess), NtTerminateProcess gets called twice. The first time it gets called specially (with the process handle passed as NULL) in order to forcefully terminate all other threads in the process. Once the thread that calls ExitProcess is the last remaining thread, the loader shuts down the user-mode aspects of the process (e.g. it calls DLL entry points for process detach). Finally, the last thread makes a regular NtTerminateProcess call (with the current-process handle instead of NULL), which actually terminates the process.

An abnormal termination just does the latter step, but it doesn't necessarily use an exit status value that clearly indicates an abnormal termination. Thus not all abnormal terminations can be identified as such. Also, nothing stops a normal termination via ExitProcess from using an NTSTATUS code. For example, the default control handler for a console process exits via ExitProcess with the status code STATUS_CONTROL_C_EXIT (0xC000_013A). This is similar to an abnormal exit, since the process is killed by the closest thing to a Unix 'signal' that Windows console applications support. Moreover, the same status code is used for a genuinely abnormal exit due to a Ctrl+Close event (i.e. the console window was closed) if the session server is forced to terminate a console process that doesn't exit gracefully in the allotted time (default 5 seconds).

New changeset 7c72383f95b0cdedf390726069428d7b69ed2597 by Victor Stinner in branch 'master':
bpo-40094: Enhance os.WIFEXITED documentation (GH-19244)
https://github.com/python/cpython/commit/7c72383f95b0cdedf390726069428d7b69ed2597

Eryk:
> The current waitpid() implementation assumes it's okay to discard the upper 8 bits of the exit status, which can lose important information.

That's a bug which is independent of this issue.

> Thus not all abnormal terminations can be identified as such. Also, nothing stops a normal termination via ExitProcess from using an NTSTATUS code.

Ok, so the current os.waitstatus_to_exitcode() design is fine. On Windows, we can just consider all exit code as a "normal" process exit code.

And there is no need to modify os.waitpid() to return a negative value for values larger than (INT_MAX >> 8). We should "just" fix os.waitstatus_to_exitcode() to accept any Python integer and simply compute "x >> 8", whereas currently the argument is casted to a C int.

I propose to fix os.waitpid() and os.waitstatus_to_exitcode() for "large" exit code on Windows in a second time.

New changeset 65a796e5272f61b42792d3a8c69686558c1872c5 by Victor Stinner in branch 'master':
bpo-40094: Add os.waitstatus_to_exitcode() (GH-19201)
https://github.com/python/cpython/commit/65a796e5272f61b42792d3a8c69686558c1872c5

sys.exit() accepts negative number and values larger than 255. I checked with strace: Python calls Linux exit_group() syscall with the value passed to sys.exit().

But then os.waitid() (waitid, not waitpid!) returns the lower 8-bits of the exit code.

In fact, the exit_group() syscall truncates the exit status:
https://github.com/torvalds/linux/blob/1a323ea5356edbb3073dc59d51b9e6b86908857d/kernel/exit.c#L895-L905

So on Linux, an exit code is always in the range [0; 255]. For example, exit_group(-1) syscall gives an exit code of 255.

TODO:

* Modify asyncio.unix_events._compute_returncode() to use waitstatus_to_exitcode(): need to update tests.
* Modify run_cgi() of http.server to log the exit code rather the exit status: use waitstatus_to_exitcode().
* Modify Tools/scripts/which.py to log the exit code using waitstatus_to_exitcode(): result of os.system('ls ' + longlist + ' ' + filename).
* Modify mailcap.test() to use waitstatus_to_exitcode(): os.system(command).
* Fix CI to get PR 19277 and PR 19278 merged.
* Decide if subprocess should reject WIFSTOPPED() or not.
* Check if the pure Python implementation of os._spawnvef() handles WIFSTOPPED() properly.
* Maybe implement timeout on Windows for test.support.wait_process().


Eryk Sun:
> FWIW, I wouldn't recommend relying on os.waitpid to get the correct process exit status in Windows. Status codes are 32 bits and generally all bits are required.

I created bpo-40138 "Windows implementation of os.waitpid() truncates the exit status (status << 8)".

See also: "Appendix E. Exit Codes With Special Meanings" section of the Bash documentation
https://tldp.org/LDP/abs/html/exitcodes.html

New changeset c8dd641b6214bdcf794bab469a51da6843feb770 by Miss Islington (bot) in branch '3.7':
bpo-40094: Enhance os.WIFEXITED documentation (GH-19244) (GH-19278)
https://github.com/python/cpython/commit/c8dd641b6214bdcf794bab469a51da6843feb770

New changeset 267afc2ab2014e1e3c6b2ff088350a69b691a544 by Miss Islington (bot) in branch '3.8':
bpo-40094: Enhance os.WIFEXITED documentation (GH-19244) (GH-19277)
https://github.com/python/cpython/commit/267afc2ab2014e1e3c6b2ff088350a69b691a544

> Decide if subprocess should reject WIFSTOPPED() or not.

This code path was added by bpo-29335.

New changeset d57cf557366584539f400db523b555296487e8f5 by Victor Stinner in branch 'master':
bpo-40094: mailcap.test() uses waitstatus_to_exitcode() (GH-19287)
https://github.com/python/cpython/commit/d57cf557366584539f400db523b555296487e8f5

New changeset 9a679a0e47d58aa73b7747d4e16140048c10baf5 by Victor Stinner in branch 'master':
bpo-40094: CGIHTTPRequestHandler logs exit code (GH-19285)
https://github.com/python/cpython/commit/9a679a0e47d58aa73b7747d4e16140048c10baf5

New changeset e7c98f08e228e9f6e139d61e3e5d0a5018a38f0b by Victor Stinner in branch 'master':
bpo-40094: Fix which.py script exit code (GH-19286)
https://github.com/python/cpython/commit/e7c98f08e228e9f6e139d61e3e5d0a5018a38f0b

See also bpo-40155: "AIX: test_builtin.test_input_no_stdout_fileno() hangs".

> TODO: Modify asyncio.unix_events._compute_returncode() to use waitstatus_to_exitcode(): need to update tests.

I created bpo-40364 for that.

"""
TODO:
* Decide if subprocess should reject WIFSTOPPED() or not.
* Check if the pure Python implementation of os._spawnvef() handles WIFSTOPPED() properly.
"""

Well, let's keep the status quo: leave os and subprocess modules unchanged. It can be revisited later if needed. My intent when I created this issue wasn't to change the behavior of other modules, just to add a new function to remove duplicated code ;-)

The initial issue has been implemented: I added os.waitstatus_to_exitcode() function to Python 3.9. It's now well documented, I close the issue.

See sub-issues like bpo-40364 (asyncio) for further cleanups.