Issue40138
Created on 2020-04-01 17:05 by vstinner, last changed 2020-04-22 16:35 by vstinner. This issue is now closed.
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 19637 | merged | vstinner, 2020-04-21 22:23 | |
| PR 19654 | merged | vstinner, 2020-04-22 14:46 | |
| PR 19655 | merged | miss-islington, 2020-04-22 15:58 | |
| Messages (5) | |||
|---|---|---|---|
| msg365498 - (view) | Author: STINNER Victor (vstinner) * | Date: 2020-04-01 17:05 | |
On Windows, the exit code is a 32-bit value. It may or may not signed depending on the function.
Unsigned in the Windows native API:
BOOL TerminateProcess(HANDLE hProcess, UINT uExitCode);
BOOL GetExitCodeProcess(HANDLE hProcess, LPDWORD lpExitCode);
Signed in the POSIX API:
intptr_t _cwait(int *termstat, intptr_t procHandle, int action);
Problem: os.waitpid() uses "status << 8" which can overflow; status is an int.
static PyObject *
os_waitpid_impl(PyObject *module, intptr_t pid, int options)
{
int status;
(...)
/* shift the status left a byte so this is more like the POSIX waitpid */
return Py_BuildValue(_Py_PARSE_INTPTR "i", res, status << 8);
}
int64_t or uint64_t should be used, or a Python object should be used, to avoid the overflow.
I just added os.waitstatus_to_exitcode() in bpo-40094 which simply does "status >> 8" on Windows. Currently, this function casts the argument to a C int and so is limited to INT_MAX. It should also be adapted to handle values larger than INT_MAX.
By the way, I'm not sure how to handle values larger than INT_MAX. The POSIX API of Windows uses a signed integer, and so convert such value as a negative value. But the native Windows API uses unsigned numbers.
It seems like using unsigned number would be better.
--
By the way, currently os.waitstatus_to_exitcode() ignores the lower 8 bits of the status. Maybe it should raise an error if lower 8 bits are not zero, and maybe also raise an exception if the number is negative?
--
See also interesting comments by Eryk Sun in bpo-40094 about this problem.
|
|||
| msg367007 - (view) | Author: STINNER Victor (vstinner) * | Date: 2020-04-22 14:30 | |
New changeset 9bee32b34e4fb3e67a88bf14d38153851d4c4598 by Victor Stinner in branch 'master': bpo-40138: Fix Windows os.waitpid() for large exit code (GH-19637) https://github.com/python/cpython/commit/9bee32b34e4fb3e67a88bf14d38153851d4c4598 |
|||
| msg367012 - (view) | Author: STINNER Victor (vstinner) * | Date: 2020-04-22 15:58 | |
New changeset b07350901cac9197aef41855d8a4d56533636b91 by Victor Stinner in branch '3.8': bpo-40138: Fix Windows os.waitpid() for large exit code (GH-19654) https://github.com/python/cpython/commit/b07350901cac9197aef41855d8a4d56533636b91 |
|||
| msg367016 - (view) | Author: miss-islington (miss-islington) | Date: 2020-04-22 16:16 | |
New changeset de5dcfa3bcabf52e43468a2b088ed71b5e5c4503 by Miss Islington (bot) in branch '3.7': bpo-40138: Fix Windows os.waitpid() for large exit code (GH-19654) https://github.com/python/cpython/commit/de5dcfa3bcabf52e43468a2b088ed71b5e5c4503 |
|||
| msg367019 - (view) | Author: STINNER Victor (vstinner) * | Date: 2020-04-22 16:35 | |
Ok, os.waitpid() is now fixed in 3.7, 3.8 and master branches, and os.waitstatus_to_exitcode() is fixed in master. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2020-04-22 16:35:49 | vstinner | set | status: open -> closed versions: + Python 3.7, Python 3.8 messages: + msg367019 resolution: fixed |
| 2020-04-22 16:16:50 | miss-islington | set | messages: + msg367016 |
| 2020-04-22 15:58:32 | miss-islington | set | nosy:
+ miss-islington pull_requests: + pull_request18981 |
| 2020-04-22 15:58:07 | vstinner | set | messages: + msg367012 |
| 2020-04-22 14:46:27 | vstinner | set | pull_requests: + pull_request18980 |
| 2020-04-22 14:30:42 | vstinner | set | messages: + msg367007 |
| 2020-04-21 22:23:02 | vstinner | set | keywords:
+ patch stage: patch review pull_requests: + pull_request18962 |
| 2020-04-01 18:16:32 | xtreak | set | nosy:
+ eryksun |
| 2020-04-01 17:05:27 | vstinner | create | |