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']

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.

New changeset 267afc2 by Miss Islington (bot) in branch '3.8':
bpo-40094: Enhance os.WIFEXITED documentation (GH-19244) (GH-19277)
267afc2

Decide if subprocess should reject WIFSTOPPED() or not.

This code path was added by bpo-29335.

New changeset d57cf55 by Victor Stinner in branch 'master':
bpo-40094: mailcap.test() uses waitstatus_to_exitcode() (GH-19287)
d57cf55

New changeset 9a679a0 by Victor Stinner in branch 'master':
bpo-40094: CGIHTTPRequestHandler logs exit code (GH-19285)
9a679a0

New changeset e7c98f0 by Victor Stinner in branch 'master':
bpo-40094: Fix which.py script exit code (GH-19286)
e7c98f0

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.

fix exitcode of process issue since python 3.8

fix exitcode of process issue since python 3.8 (#87)

Navigation Menu

Repository navigation

Add os.waitstatus_to_exitcode() function #84275

17 remaining items

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Add os.waitstatus_to_exitcode() function #84275

Description

Activity

vstinner commented on Mar 27, 2020

vstinner commented on Mar 28, 2020

vstinner commented on Mar 29, 2020

vstinner commented on Mar 29, 2020

vstinner commented on Mar 29, 2020

vstinner commented on Mar 30, 2020

vstinner commented on Mar 30, 2020

17 remaining items

vstinner commented on Apr 1, 2020

vstinner commented on Apr 1, 2020

vstinner commented on Apr 2, 2020

vstinner commented on Apr 2, 2020

vstinner commented on Apr 2, 2020

vstinner commented on Apr 2, 2020

vstinner commented on Apr 22, 2020

vstinner commented on Apr 22, 2020

vstinner commented on Apr 22, 2020

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions