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: threading.Lock.acquire(timeout) should use sem_clockwait(CLOCK_MONOTONIC)
Type: behavior Stage: resolved
Components: Interpreter Core Versions: Python 3.11, Python 3.10, Python 3.9
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: methane, mikecrowe, miss-islington, vstinner, wocket
Priority: normal Keywords: patch

Created on 2020-09-04 08:34 by wocket, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Files
File name Uploaded Description Edit
time_test.py wocket, 2020-09-04 08:34
Pull Requests
URL Status Linked Edit
PR 28629 merged vstinner, 2021-09-29 16:30
PR 28642 merged vstinner, 2021-09-30 00:36
PR 28643 merged vstinner, 2021-09-30 01:10
PR 28644 merged vstinner, 2021-09-30 01:25
PR 28647 merged vstinner, 2021-09-30 08:51
PR 28662 merged vstinner, 2021-09-30 21:07
PR 28671 merged vstinner, 2021-10-01 08:20
PR 28672 closed vstinner, 2021-10-01 08:32
PR 28673 merged vstinner, 2021-10-01 09:25
PR 28674 merged vstinner, 2021-10-01 10:26
PR 28676 merged vstinner, 2021-10-01 10:59
PR 28683 merged miss-islington, 2021-10-01 16:23
PR 28962 merged vstinner, 2021-10-14 23:21
Messages (22)
msg376338 - (view) Author: Jonas Norling (wocket) Date: 2020-09-04 08:34
The timeout for threading.Lock, threading.Condition, etc, is not using a monotonic clock — it is affected if the system time (realtime clock) is set.

The attached program can be used to show the problem. It is expected to print "Took 2.000 s" repeatedly, but if run with permissions to set the system time, it prints:
$ sudo ./time_test.py
Took 2.400 s
Took 1.657 s
Took 2.044 s
Took 2.401 s
...

(the 1.6 s time can be explained by NTP correcting the clock)

There are already a number of closed bugs for this and related issues: bpo 23428, bpo 31267, bpo 35747.

This happens in Python 3.7.7 (ARM32, Yocto Warrior), Python 3.8.2 (AMD64, Ubuntu Linux 20.04) and Python v3.9.0rc1 (AMD64, Ubuntu Linux 20.04).
msg376340 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-09-04 08:49
bpo-23428 modified the pthread implementation of conditional variable to use pthread_condattr_setclock(&ca, CLOCK_MONOTONIC) is available: commit 001fee14e0f2ba5f41fb733adc69d5965925a094. The change should be part of Python 3.8.

What is your sys.thread_info value? Example on Fedora 32 with Python 3.8.5:

>>> sys.thread_info
sys.thread_info(name='pthread', lock='semaphore', version='NPTL 2.31')

Sadly, the semaphore implementation doesn't use monotonic clock. See glibc issues:

* https://sourceware.org/bugzilla/show_bug.cgi?id=14717
* https://bugzilla.kernel.org/show_bug.cgi?id=112521
msg376394 - (view) Author: Jonas Norling (wocket) Date: 2020-09-04 18:55
sys.thread_info = sys.thread_info(name='pthread', lock='semaphore', version='NPTL 2.31') on my system. Looking at the source I think the semaphore implementation will be used on all modern Linux systems.

In my tests it works as expected on a Macintosh (3.8.5 with lock='mutex+cond') and also if I force a Linux build to use the mutex+cond implementation by defining HAVE_BROKEN_POSIX_SEMAPHORES.

Doesn't look like those glibc and Linux bug reports will get any attention anytime soon. I will find a workaround instead :-/
msg399405 - (view) Author: Mike Crowe (mikecrowe) Date: 2021-08-11 16:11
glibc v2.30 onwards provides sem_clockwait which can wait on either CLOCK_MONOTONIC or CLOCK_REALTIME. I failed to notice that https://sourceware.org/bugzilla/show_bug.cgi?id=14717 existed until today. :(
msg402925 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-09-30 00:11
New changeset 09796f2f142fdb1214f34a3ca917959ecb32a88b by Victor Stinner in branch 'main':
bpo-41710: Add _PyTime_AsTimespec_clamp() (GH-28629)
https://github.com/python/cpython/commit/09796f2f142fdb1214f34a3ca917959ecb32a88b
msg402926 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-09-30 01:07
New changeset d62d925823b005c33b432e527562b573a3a89635 by Victor Stinner in branch 'main':
bpo-41710: Add pytime_add() and pytime_mul() (GH-28642)
https://github.com/python/cpython/commit/d62d925823b005c33b432e527562b573a3a89635
msg402931 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-09-30 01:50
New changeset 0231b6da45b610d33ee4e99bf190e31488d6ab26 by Victor Stinner in branch 'main':
bpo-41710: Fix building pytime.c on Windows (GH-28644)
https://github.com/python/cpython/commit/0231b6da45b610d33ee4e99bf190e31488d6ab26
msg402939 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-09-30 08:17
New changeset 37b8294d6295ca12553fd7c98778be71d24f4b24 by Victor Stinner in branch 'main':
bpo-41710: PyThread_acquire_lock_timed() clamps the timout (GH-28643)
https://github.com/python/cpython/commit/37b8294d6295ca12553fd7c98778be71d24f4b24
msg402942 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-09-30 09:23
New changeset b34dd58fee707b8044beaf878962a6fa12b304dc by Victor Stinner in branch 'main':
bpo-41710: Document _PyTime_t API in pytime.h (GH-28647)
https://github.com/python/cpython/commit/b34dd58fee707b8044beaf878962a6fa12b304dc
msg402987 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-09-30 21:20
On Unix, PyCOND_TIMEDWAIT() is implemented with pthread_cond_timedwait(). If pthread_condattr_setclock() is available, it uses CLOCK_MONOTONIC. Otherwise, it uses CLOCK_REALTIME.

The glibc 2.30 adds pthread_cond_clockwait() which could be used to use CLOCK_MONOTONIC. But if pthread_cond_clockwait() is available (glibc 2.30 or newer), it expects that pthread_condattr_setclock() is also available. So I'm not sure that it's worth it to change PyCOND_TIMEDWAIT().

See the _PyThread_cond_after() function which computes an absolute timestamp (timespec) from a relative timeout in microseconds.
msg402995 - (view) Author: Mike Crowe (mikecrowe) Date: 2021-10-01 07:26
vstinner wrote:
> The glibc 2.30 adds pthread_cond_clockwait() which could be used to use
> CLOCK_MONOTONIC. But if pthread_cond_clockwait() is available (glibc 
> 2.30 or newer), it expects that pthread_condattr_setclock() is also 
> available. So I'm not sure that it's worth it to change 
> PyCOND_TIMEDWAIT().

That's correct. The only time that pthread_cond_clockwait is essential is if you don't know which clock you're going to need to use for the wait at the time of condition variable creation. (This is true for a generic condition variable wrapper that can be passed absolute timeouts using different clocks like the C++ std::condition_variable.)

However, sem_clockwait is the only way to wait on a semaphore using CLOCK_MONOTONIC, so it is worth using that rather than sem_timedwait if it's available. Similarly for pthread_mutex_clocklock.
msg402997 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-10-01 07:55
New changeset 1ee0f94d16f150356a4b9b0a39d44ba1d2d5b9fc by Victor Stinner in branch 'main':
bpo-41710: PyThread_acquire_lock_timed() uses sem_clockwait() (GH-28662)
https://github.com/python/cpython/commit/1ee0f94d16f150356a4b9b0a39d44ba1d2d5b9fc
msg403001 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-10-01 08:00
With sem_clockwait(CLOCK_MONOTONIC) on Fedora 34 (glibc-2.33-20.fc34.x86_64, Linux kernel 5.13.19-200.fc34.x86_64), time_test.py now works as expected:

$ sudo ./python time_test.py
Took 2.000 s
Took 2.000 s
Took 2.000 s
Took 2.000 s
Took 2.000 s
Took 2.000 s
(...)
msg403003 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-10-01 08:53
> There are already a number of closed bugs for this and related issues: bpo 23428, bpo 31267, bpo 35747.

Fixed bpo-12822 modified threading.Condition.wait(timeout) to use a monotonic clock: use pthread_condattr_setclock(CLOCK_MONOTONIC). bpo-23428, bpo-31267 and bpo-35747 are duplicates of bpo-12822.

This issue is about threading.Lock.acquire(timeout) which has a different implementation.
msg403009 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-10-01 11:03
New changeset 98d282700221234157159df4af76423d89490ad9 by Victor Stinner in branch 'main':
bpo-41710: Fix PY_TIMEOUT_MAX on Windows (GH-28673)
https://github.com/python/cpython/commit/98d282700221234157159df4af76423d89490ad9
msg403011 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-10-01 11:29
New changeset 54957f16a63ecb6b15f77b01fa7c55ada892604a by Victor Stinner in branch 'main':
bpo-41710: gc_collect_main() uses _PyTime_GetPerfCounter() (GH-28676)
https://github.com/python/cpython/commit/54957f16a63ecb6b15f77b01fa7c55ada892604a
msg403012 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-10-01 11:29
New changeset 833fdf126c8fe77fd17e8a8ffbc5c571b3bf64bd by Victor Stinner in branch 'main':
bpo-41710: Add private _PyDeadline_Get() function (GH-28674)
https://github.com/python/cpython/commit/833fdf126c8fe77fd17e8a8ffbc5c571b3bf64bd
msg403021 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-10-01 16:22
New changeset 6df8c327532627d6a99991993c52e8e4a9b34968 by Victor Stinner in branch '3.10':
bpo-41710: PyThread_acquire_lock_timed() uses sem_clockwait() (GH-28671)
https://github.com/python/cpython/commit/6df8c327532627d6a99991993c52e8e4a9b34968
msg403022 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-10-01 16:51
New changeset 0e1aeab5d7de3f328876aea8ccabbc6db146a883 by Miss Islington (bot) in branch '3.9':
bpo-41710: PyThread_acquire_lock_timed() uses sem_clockwait() (GH-28671) (GH-28683)
https://github.com/python/cpython/commit/0e1aeab5d7de3f328876aea8ccabbc6db146a883
msg403023 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-10-01 16:52
Jonas Norling: Thanks for the bug report! It's now fixed in 3.9, 3.10 and main branches.
msg403962 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-10-14 23:49
New changeset 03bbc6066ff40c62edd57612be9150dcf1b123c8 by Victor Stinner in branch 'main':
bpo-41710: Fix What's New Entry credit (GH-28962)
https://github.com/python/cpython/commit/03bbc6066ff40c62edd57612be9150dcf1b123c8
msg407802 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2021-12-06 13:27
bpo-33632 "undefined behaviour: signed integer overflow in threadmodule.c" has been fixed by this change:

New changeset 833fdf126c8fe77fd17e8a8ffbc5c571b3bf64bd by Victor Stinner in branch 'main':
bpo-41710: Add private _PyDeadline_Get() function (GH-28674)
https://github.com/python/cpython/commit/833fdf126c8fe77fd17e8a8ffbc5c571b3bf64bd
History
Date User Action Args
2022-04-11 14:59:35adminsetgithub: 85876
2021-12-06 13:27:11vstinnersetmessages: + msg407802
2021-12-06 13:26:08vstinnerlinkissue33632 superseder
2021-10-14 23:49:40vstinnersetmessages: + msg403962
2021-10-14 23:21:18vstinnersetpull_requests: + pull_request27251
2021-10-01 16:52:29vstinnersetstatus: open -> closed
versions: + Python 3.9, Python 3.10
messages: + msg403023

resolution: fixed
stage: patch review -> resolved
2021-10-01 16:51:27vstinnersetmessages: + msg403022
2021-10-01 16:23:05miss-islingtonsetnosy: + miss-islington
pull_requests: + pull_request27047
2021-10-01 16:22:57vstinnersetmessages: + msg403021
2021-10-01 11:29:29vstinnersetmessages: + msg403012
2021-10-01 11:29:13vstinnersetmessages: + msg403011
2021-10-01 11:03:06vstinnersetmessages: + msg403009
2021-10-01 10:59:31vstinnersetpull_requests: + pull_request27042
2021-10-01 10:26:40vstinnersetpull_requests: + pull_request27039
2021-10-01 09:25:58vstinnersetpull_requests: + pull_request27038
2021-10-01 08:53:13vstinnersetmessages: + msg403003
2021-10-01 08:32:51vstinnersetpull_requests: + pull_request27037
2021-10-01 08:20:43vstinnersetpull_requests: + pull_request27036
2021-10-01 08:00:08vstinnersettitle: acquire(timeout) of threading.Lock and threading.Condition is affected by jumps in system time: Python should use sem_clockwait(CLOCK_MONOTONIC) -> threading.Lock.acquire(timeout) should use sem_clockwait(CLOCK_MONOTONIC)
messages: + msg403001
versions: + Python 3.11, - Python 3.8
2021-10-01 07:55:36vstinnersetmessages: + msg402997
2021-10-01 07:26:31mikecrowesetmessages: + msg402995
2021-09-30 21:20:06vstinnersetmessages: + msg402987
2021-09-30 21:07:53vstinnersetpull_requests: + pull_request27027
2021-09-30 09:23:13vstinnersetmessages: + msg402942
2021-09-30 08:51:16vstinnersetpull_requests: + pull_request27015
2021-09-30 08:17:02vstinnersetmessages: + msg402939
2021-09-30 01:50:41vstinnersetmessages: + msg402931
2021-09-30 01:25:02vstinnersetpull_requests: + pull_request27012
2021-09-30 01:10:43vstinnersetpull_requests: + pull_request27011
2021-09-30 01:07:19vstinnersetmessages: + msg402926
2021-09-30 00:36:28vstinnersetpull_requests: + pull_request27010
2021-09-30 00:11:49vstinnersetmessages: + msg402925
2021-09-29 16:30:13vstinnersetkeywords: + patch
stage: patch review
pull_requests: + pull_request26999
2021-09-13 14:59:05vstinnersettitle: Timeout is affected by jumps in system time -> acquire(timeout) of threading.Lock and threading.Condition is affected by jumps in system time: Python should use sem_clockwait(CLOCK_MONOTONIC)
2021-08-11 16:11:17mikecrowesetnosy: + mikecrowe
messages: + msg399405
2020-09-04 18:55:50wocketsetmessages: + msg376394
2020-09-04 08:49:30vstinnersetnosy: + vstinner, methane
messages: + msg376340
2020-09-04 08:34:54wocketcreate