threading.Condition.wait(timeout) should use a monotonic clock: use pthread_condattr_setclock(CLOCK_MONOTONIC)

[methane]

BPO	12822
Nosy	@malemburg, @pitrou, @vstinner, @methane
PRs	bpo-12822: use monotonic clock for condvar if possible #11723 bpo-12822: use monotonic clock for condvar if possible #11723
Files	use_monotonic_clock.patch

^{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 2019-02-20.01:00:28.338>
created_at = <Date 2011-08-23.07:43:36.382>
labels = ['interpreter-core', 'type-bug', '3.8']
title = 'threading.Condition.wait(timeout) should use a monotonic clock: use pthread_condattr_setclock(CLOCK_MONOTONIC)'
updated_at = <Date 2021-10-01.08:52:42.541>
user = 'https://github.com/methane'

bugs.python.org fields:

activity = <Date 2021-10-01.08:52:42.541>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2019-02-20.01:00:28.338>
closer = 'methane'
components = ['Interpreter Core']
creation = <Date 2011-08-23.07:43:36.382>
creator = 'methane'
dependencies = []
files = ['23013']
hgrepos = []
issue_num = 12822
keywords = ['patch']
message_count = 13.0
messages = ['142789', '142791', '142856', '142858', '142859', '148525', '148528', '148529', '155837', '334621', '334623', '336032', '336061']
nosy_count = 5.0
nosy_names = ['lemburg', 'pitrou', 'vstinner', 'methane', 'neologix']
pr_nums = ['11723', '11723']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue12822'
versions = ['Python 3.8']

[methane]

Using CLOCK_MONOTONIC is better than CLOCK_REALTIME (default) for GIL
because settimeofday() may break the pthread_cond_timedwait().

Attached patch uses CLOCK_MONOTONIC and clock_gettime. But I don't know
how to write appropriate configure script.
"-lrt" is also needed to use clock_gettime() but I don't know how to add
it to LIBS.

[vstinner]

See also bpo-10278.

[neologix]

Using CLOCK_MONOTONIC is better than CLOCK_REALTIME (default) for GIL
because settimeofday() may break the pthread_cond_timedwait().

Indeed.
A couple remarks:

• regular locks and conditions variables exposed by the threading module suffer from the same problem
• POSIX semaphores are also affected, but you can't select an alternative clock source
• actually, CLOCK_MONOTONIC is affected by NTP adjustements: while it's guaranteed not to go backward, its rate can be affected

Did you really encounter this problem, or is this just a theoretical concern?

[pitrou]

The patch is ok on the principle, but we do need a check that CLOCK_MONOTONIC is supported at build time.

[malemburg]

Antoine Pitrou wrote:

The patch is ok on the principle, but we do need a check that CLOCK_MONOTONIC is supported at build time.

I think we need both: a check at build time to avoid
compiler errors and a check at runtime whether the deployment
platform supports the clock, plus a fallback solution in case
it is not available.

[vstinner]

The patch is ok on the principle, but we do need a check
that CLOCK_MONOTONIC is supported at build time.

timemodule.c is now using "#ifdef CLOCK_MONOTONIC".

[pitrou]

Marc-Andre is right, a runtime check is probably also needed.
(for example with mismatching kernel/libc)

[vstinner]

pthread_condattr_setclock() result should be checked.

"The pthread_condattr_setclock() function may fail if:

EINVAL The value specified by clock_id does not refer to a known clock, or is a CPU-time clock."

[vstinner]

See also bpo-14222. Python 3.3 has a new time.steady() function.

[methane]

ref: https://bugs.chromium.org/p/webrtc/issues/detail?id=9269
macOS and iOS don't support pthread_condattr_setclock yet.

[vstinner]

See also bpo-23428: "Use the monotonic clock for thread conditions on POSIX platforms".

[methane]

New changeset 001fee1 by Inada Naoki in branch 'master':
bpo-12822: use monotonic clock for condvar if possible (GH-11723)
001fee1

[vstinner]

Thanks INADA-san for fixing this old issue!