This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: undefined behaviour: signed integer overflow in threadmodule.c
Type: behavior Stage: resolved
Components: Extension Modules Versions: Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder: threading.Lock.acquire(timeout) should use sem_clockwait(CLOCK_MONOTONIC)
View: 41710
Assigned To: Nosy List: ZackerySpytz, hongweipeng, martin.panter, p-ganssle, pitrou, vstinner
Priority: normal Keywords: patch

Created on 2018-05-24 08:39 by pitrou, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 12729 closed ZackerySpytz, 2019-04-08 14:34
Messages (9)
msg317545 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2018-05-24 08:39
Modules/_threadmodule.c:52:47: runtime error: signed integer overflow: 2387971499048 + 9223372036000000000 cannot be represented in type 'long'
msg317554 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2018-05-24 11:00
Looks like this is what my thread.patch was fixing in <https://bugs.python.org/issue1621#msg271057>. You’re welcome to use my patch, but I won’t have time to work on it myself.
msg317556 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-05-24 11:18
> Modules/_threadmodule.c:52:47: runtime error: signed integer overflow: 2387971499048 + 9223372036000000000 cannot be represented in type 'long'

How do you reproduce the issue? The thread module should limit the maximum timeout to PY_TIMEOUT_MAX. Maybe PY_TIMEOUT_MAX is too big?
msg339644 - (view) Author: Zackery Spytz (ZackerySpytz) * (Python triager) Date: 2019-04-08 14:37
I've created a PR based on Martin Panter's patch.
msg340184 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2019-04-14 06:05
Victor, if you run the test suite, one of the test cases should trigger the overflow. I used to compile with Undefined Behaviour Sanitizer to print messages when these errors occur; see <https://bugs.python.org/issue1621#msg271118> for my setup at the time. I presume Antoine did something similar.

I do not remember, but suspect the test case might be the following lines of “BaseLockTests.test_timeout” in Lib/test/lock_tests.py, testing a fraction of a second less than PY_TIMEOUT_MAX:

# TIMEOUT_MAX is ok
lock.acquire(timeout=TIMEOUT_MAX)

Perhaps reducing PY_TIMEOUT_MAX by a few centuries would be one way to avoid the problem. In my patch I avoided the problem by rearranging the arithmetic, so that the timeout value is only compared and reduced, never added.
msg340255 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2019-04-15 10:28
In short, a+b can overflow, but a-b cannot?
msg340284 - (view) Author: Paul Ganssle (p-ganssle) * (Python committer) Date: 2019-04-15 15:09
> In short, a+b can overflow, but a-b cannot?

I think it's more that by always checking the elapsed time against `now() - starttime`, you never need to represent the time at which the timeout should happen - which may be so far in the future that it causes a signed overflow.
msg407712 - (view) Author: hongweipeng (hongweipeng) * Date: 2021-12-05 15:51
I think PR https://github.com/python/cpython/pull/28674 has resolved this issue.
msg407800 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-12-06 13:26
> I think PR https://github.com/python/cpython/pull/28674 has resolved this issue.

You're right.

_threadmodule.c now uses _PyDeadline_Init() which calls _PyTime_Add(), and _PyTime_Add() prevents integer overflows; Extract of its implementation:

// Compute t1 + t2. Clamp to [_PyTime_MIN; _PyTime_MAX] on overflow.
static inline int
pytime_add(_PyTime_t *t1, _PyTime_t t2)
{
    if (t2 > 0 && *t1 > _PyTime_MAX - t2) {
        *t1 = _PyTime_MAX;
        return -1;
    }
    else if (t2 < 0 && *t1 < _PyTime_MIN - t2) {
        *t1 = _PyTime_MIN;
        return -1;
    }
    else {
        *t1 += t2;
        return 0;
    }
}
History
Date User Action Args
2022-04-11 14:59:00adminsetgithub: 77813
2022-03-24 15:37:26iritkatriellinkissue37665 superseder
2021-12-06 13:26:08vstinnersetstatus: open -> closed
superseder: threading.Lock.acquire(timeout) should use sem_clockwait(CLOCK_MONOTONIC)
messages: + msg407800

resolution: fixed
stage: patch review -> resolved
2021-12-05 15:51:21hongweipengsetnosy: + hongweipeng
messages: + msg407712
2019-04-15 15:09:11p-gansslesetmessages: + msg340284
2019-04-15 10:28:13vstinnersetmessages: + msg340255
2019-04-14 06:05:46martin.pantersetmessages: + msg340184
2019-04-08 15:16:23p-gansslesetnosy: + p-ganssle
2019-04-08 14:37:00ZackerySpytzsetversions: - Python 3.6
nosy: + ZackerySpytz

messages: + msg339644

components: + Extension Modules, - Library (Lib)
2019-04-08 14:34:15ZackerySpytzsetkeywords: + patch
stage: patch review
pull_requests: + pull_request12652
2018-05-24 11:18:39vstinnersetnosy: + vstinner
messages: + msg317556
2018-05-24 11:00:27martin.pantersetnosy: + martin.panter
messages: + msg317554
2018-05-24 08:39:55pitroucreate