Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-41710: PyThread_acquire_lock_timed() uses sem_clockwait() #28662

Merged
merged 3 commits into from Oct 1, 2021

Conversation

@vstinner
Copy link
Member

@vstinner vstinner commented Sep 30, 2021

On Unix, if the sem_clockwait() function is available in the C
library (glibc 2.30 and newer), the threading.Lock.acquire() method
now uses the monotonic clock (time.CLOCK_MONOTONIC) for the timeout,
rather than using the system clock (time.CLOCK_REALTIME), to not be
affected by system clock changes.

configure now checks if the sem_clockwait() function is available.

https://bugs.python.org/issue41710

On Unix, if the sem_clockwait() function is available in the C
library (glibc 2.30 and newer), the threading.Lock.acquire() method
now uses the monotonic clock (time.CLOCK_MONOTONIC) for the timeout,
rather than using the system clock (time.CLOCK_REALTIME), to not be
affected by system clock changes.

configure now checks if the sem_clockwait() function is available.
deadline = _PyTime_GetMonotonicClock() + timeout;
}

while (1) {
if (timeout > 0) {
#ifdef HAVE_SEM_CLOCKWAIT
struct timespec abs_timeout;
_PyTime_AsTimespec_clamp(deadline, &abs_timeout);
Copy link
Member Author

@vstinner vstinner Sep 30, 2021

I don't know if sem_clockwait() timeout argument can be modified by the function, for example if the wait is interrupted by a signal. In case of doubt, I prefer to compute abs_timeout before each call.

Copy link
Member Author

@vstinner vstinner Sep 30, 2021

In the same file, pthread_cond_timedwait() absolute timeout is only computed once outside the loop. I understand that we can also compute abs_timeout once outside the loop.

The new implementation of time.sleep(), pysleep() function, now also uses an absolute timeout (timespec) computed once outside the loop. It uses clock_nanosleep() or nanosleep(). These functions have an argument for the remaining time, it's not used by pysleep().

Copy link
Member Author

@vstinner vstinner Sep 30, 2021

sem_clockwait() documentation doesn't say that the absolute timeout is modified, which confirms that it can be only computed once:
https://www.gnu.org/software/libc/manual/html_node/Waiting-with-Explicit-Clocks.html

#ifdef HAVE_SEM_CLOCKWAIT
struct timespec abs_timeout;
_PyTime_AsTimespec_clamp(deadline, &abs_timeout);
status = fix_status(sem_clockwait(thelock, CLOCK_MONOTONIC, &abs_timeout));
Copy link
Member Author

@vstinner vstinner Sep 30, 2021

I don't know if sem_clockwait() can fail with ENOSYS if Python is built with a recent glibc and then run with an older glibc or a different kernel version.

Should we attempt to fallback to sem_timedwait() if sem_clockwait() "doesn't work"? How can we determine if sem_clockwait() "doesn't work"? So far, I failed to find a sem_clockwait() documentation or manual page.

Copy link
Member Author

@vstinner vstinner Sep 30, 2021

In case of doubt, I prefer to leave the code as it is, and only implement a fallback once we can test it on a real system.

@vstinner
Copy link
Member Author

@vstinner vstinner commented Sep 30, 2021

@vstinner
Copy link
Member Author

@vstinner vstinner commented Sep 30, 2021

gpshead
gpshead approved these changes Oct 1, 2021
@@ -486,6 +503,9 @@ PyThread_acquire_lock_timed(PyThread_type_lock lock, PY_TIMEOUT_T microseconds,
break;
}

// sem_clockwait() uses an absolution timeout, there is no need
Copy link
Member

@gpshead gpshead Oct 1, 2021

s/absolution/absolute

Copy link
Member Author

@vstinner vstinner Oct 1, 2021

Nobody expects the Spanish Absolution!

@vstinner
Copy link
Member Author

@vstinner vstinner commented Oct 1, 2021

@gpshead @corona10: Thanks for your reviews ;-)

@vstinner vstinner merged commit 1ee0f94 into python:main Oct 1, 2021
12 checks passed
@vstinner vstinner deleted the sem_clockwait branch Oct 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants