[3.10] bpo-41710: PyThread_acquire_lock_timed() uses sem_clockwait() #28671
Conversation
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.
|
@gpshead @corona10: Do you think that it's worth it to backport the sem_clockwait() change from main the 3.9 and 3.10? I see it as a bugfix for https://bugs.python.org/issue41710 bug. In the main branch, I wrote pytime_add() and _PyTime_AsTimespec_clamp() to handle timeout and clock values close to the C type limits. It should make the code safer. I made tons of changes in pytime.c for that. I don't think that it's worth it to backport these enhancements. Just don't use crazy timeouts :-D The current code doesn't clamp values, so this change doesn't add any regression. In Modules/_threadmodule.c, lock_acquire_parse_args() makes sure that the timeout is less than PY_TIMEOUT_MAX. Moreover, PyThread_acquire_lock_timed() has a sanity check: In main, I removed the Py_FatalError() call thanks to the new _PyTime_AsTimespec_clamp() function. By the way, I documented pytime.c in pytime.h ;-) (in the main branch) |
I am +0 on this. |
|
I think it is reasonable as a bugfix. sem_clockwait appears to be a pure libc only function, not a new syscall requiring new os kernel support so the risk seems low. When compiled on a system with headers defining it the binary already requires that min. library version to run. So it should be fine. |
Yeah, when I wrote my change for the main branch, I read the sem_clockwait() source code in the glibc. It shares code with other semaphore functions and it doesn't seem to be a new syscall. At least, I failed to find a specific syscall call in the code or using strace. |
|
Thanks @vstinner for the PR 🌮🎉.. I'm working now to backport this PR to: 3.9. |
|
GH-28683 is a backport of this pull request to the 3.9 branch. |
…GH-28671) 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. (cherry picked from commit 6df8c32) Co-authored-by: Victor Stinner <[email protected]>
…) (GH-28683) 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. (cherry picked from commit 6df8c32) Co-authored-by: Victor Stinner <[email protected]>
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