GeezerVisonThe World Wide Web, restored to 800 pixels of glory. | |||||||||||||||||||||||||||||||||||||
| Home|About|Gaming cabinet|Archives|How it works|Starter sites | |||||||||||||||||||||||||||||||||||||
GeezerVison is showing github.com — bpo-41710: PyThread_acquire_lock_timed() uses sem_clockwait() by vstinner · Pull Request #28662 · python/cpython Conversation
|
| if (timeout > 0) { | ||
| #ifdef HAVE_SEM_CLOCKWAIT | ||
| struct timespec abs_timeout; | ||
| _PyTime_AsTimespec_clamp(deadline, &abs_timeout); |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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().
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| #ifdef HAVE_SEM_CLOCKWAIT | ||
| struct timespec abs_timeout; | ||
| _PyTime_AsTimespec_clamp(deadline, &abs_timeout); | ||
| status = fix_status(sem_clockwait(thelock, CLOCK_MONOTONIC, &abs_timeout)); |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
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.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
| break; | ||
| } | ||
|
|
||
| // sem_clockwait() uses an absolution timeout, there is no need |
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
s/absolution/absolute
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
Nobody expects the Spanish Absolution!
Sorry, something went wrong.
There was an error while loading. Please reload this page.
Sorry, something went wrong.
There was an error while loading. Please reload this page.
There was an error while loading. Please reload this page.
Successfully merging this pull request may close these issues.
There was an error while loading. Please reload this page.