Add os.waitstatus_to_exitcode() function

[vstinner]

BPO	40094
Nosy	@vstinner, @eryksun, @miss-islington, @isidentical
PRs	bpo-40094: Add os.waitstatus_to_exitcode() #19201 bpo-40094: Enhance os.WIFEXITED documentation #19244 bpo-40094: Add test.support.wait_process() #19254 bpo-40094: Enhance fork and wait tests #19259 bpo-40094: Enhance threading tests #19260 bpo-31160: Fix race condition in test_os.PtyTests #19263 bpo-40094: Enhance _bootsubprocess #19264 bpo-40094: Add run_command() to setup.py #19266 bpo-40094: Add missing import to wait_process() #19268 [3.8] bpo-40094: Enhance os.WIFEXITED documentation (GH-19244) #19277 [3.7] bpo-40094: Enhance os.WIFEXITED documentation (GH-19244) #19278 bpo-40094: CGIHTTPRequestHandler logs exit code #19285 bpo-40094: Fix which.py script exit code #19286 bpo-40094: mailcap.test() uses waitstatus_to_exitcode() #19287

^{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 2020-04-22.16:55:12.178>
created_at = <Date 2020-03-27.23:29:47.109>
labels = ['library', '3.9']
title = 'Add os.waitstatus_to_exitcode() function'
updated_at = <Date 2020-04-22.16:55:12.177>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2020-04-22.16:55:12.177>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2020-04-22.16:55:12.178>
closer = 'vstinner'
components = ['Library (Lib)']
creation = <Date 2020-03-27.23:29:47.109>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 40094
keywords = ['patch']
message_count = 33.0
messages = ['365195', '365223', '365258', '365282', '365284', '365316', '365341', '365393', '365406', '365409', '365412', '365413', '365429', '365431', '365435', '365443', '365456', '365467', '365493', '365494', '365495', '365499', '365528', '365529', '365530', '365532', '365533', '365555', '365556', '365588', '367022', '367023', '367024']
nosy_count = 4.0
nosy_names = ['vstinner', 'eryksun', 'miss-islington', 'BTaskaya']
pr_nums = ['19201', '19244', '19254', '19259', '19260', '19263', '19264', '19266', '19268', '19277', '19278', '19285', '19286', '19287']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue40094'
versions = ['Python 3.9']

[vstinner]

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?

[vstinner]

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()

[vstinner]

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 ;-)

[vstinner]

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

[vstinner]

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.

[vstinner]

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.

[vstinner]

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