bpo-30765: Avoid blocking when PyThread_acquire_lock() is asked not to #2403
Conversation
…o lock This is especially important if PyThread_acquire_lock() is called reentrantly (for example from a signal handler).
There was a problem hiding this comment.
The current code doesn't compute correctly the timeout when pthread_cond_timedwait() is interrupted by a signal. We should recompute the timeout using a deadline.
Something like select.select():
if (tvp)
deadline = _PyTime_GetMonotonicClock() + timeout;
do {
... use tvp
if (errno != EINTR)
break;
/* select() was interrupted by a signal */
if (PyErr_CheckSignals())
goto finally;
if (tvp) {
timeout = deadline - _PyTime_GetMonotonicClock();
if (timeout < 0) {
n = 0;
break;
}
_PyTime_AsTimeval_noraise(timeout, &tv, _PyTime_ROUND_CEILING);
/* retry select() with the recomputed timeout */
}
} while (1);
|
Probably, but that's unrelated to this PR (and much less severe, IMHO). |
Python/thread_pthread.h
Outdated
| if (microseconds == 0) { | ||
| status = pthread_mutex_trylock( &thelock->mut ); | ||
| if (status != EBUSY) | ||
| CHECK_STATUS_PTHREAD("pthread_mutex_trylock[1]"); |
There was a problem hiding this comment.
I don't understand how these locks work. Can you please explain in a comment what happens when we fail to lock the mutex? Is it safe to call pthread_mutex_unlock() anyway?
There was a problem hiding this comment.
You're right that we may want to fail early when taking the mutex fails.
Right :-) |
I opened http://bugs.python.org/issue30768: PyThread_acquire_lock_timed() should recompute the timeout when interrupted by a signal. |
|
@Haypo, are you ok with the latest changes? |
pitrou
added
needs backport to 2.7
type-bug
… not to (pythonGH-2403) * bpo-30765: Avoid blocking when PyThread_acquire_lock() is asked not to lock This is especially important if PyThread_acquire_lock() is called reentrantly (for example from a signal handler). * Update 2017-06-26-14-29-50.bpo-30765.Q5iBmf.rst * Avoid core logic when taking the mutex failed (cherry picked from commit f84ac42)
|
GH-2418 is a backport of this pull request to the 3.6 branch. |
… not to (pythonGH-2403) * bpo-30765: Avoid blocking when PyThread_acquire_lock() is asked not to lock This is especially important if PyThread_acquire_lock() is called reentrantly (for example from a signal handler). * Update 2017-06-26-14-29-50.bpo-30765.Q5iBmf.rst * Avoid core logic when taking the mutex failed. (cherry picked from commit f84ac42)
|
GH-2419 is a backport of this pull request to the 3.5 branch. |
… not to (pythonGH-2403) * bpo-30765: Avoid blocking when PyThread_acquire_lock() is asked not to lock This is especially important if PyThread_acquire_lock() is called reentrantly (for example from a signal handler). * Update 2017-06-26-14-29-50.bpo-30765.Q5iBmf.rst * Avoid core logic when taking the mutex failed. (cherry picked from commit f84ac42)
|
GH-2420 is a backport of this pull request to the 2.7 branch. |
… not to (GH-2403) (#2418) * bpo-30765: Avoid blocking when PyThread_acquire_lock() is asked not to lock This is especially important if PyThread_acquire_lock() is called reentrantly (for example from a signal handler). * Update 2017-06-26-14-29-50.bpo-30765.Q5iBmf.rst * Avoid core logic when taking the mutex failed (cherry picked from commit f84ac42)
… not to (GH-2403) (#2419) * bpo-30765: Avoid blocking when PyThread_acquire_lock() is asked not to lock This is especially important if PyThread_acquire_lock() is called reentrantly (for example from a signal handler). * Update 2017-06-26-14-29-50.bpo-30765.Q5iBmf.rst * Avoid core logic when taking the mutex failed. (cherry picked from commit f84ac42)
… not to (GH-2403) (#2420) * [2.7] bpo-30765: Avoid blocking when PyThread_acquire_lock() is asked not to (GH-2403) * bpo-30765: Avoid blocking when PyThread_acquire_lock() is asked not to lock This is especially important if PyThread_acquire_lock() is called reentrantly (for example from a signal handler). * Update 2017-06-26-14-29-50.bpo-30765.Q5iBmf.rst * Avoid core logic when taking the mutex failed. (cherry picked from commit f84ac42) * Remove test undef
| } | ||
| if (success == PY_LOCK_ACQUIRED) thelock->locked = 1; | ||
| status = pthread_mutex_unlock( &thelock->mut ); | ||
| CHECK_STATUS_PTHREAD("pthread_mutex_unlock[1]"); | ||
|
|
||
| if (error) success = PY_LOCK_FAILURE; |
There was a problem hiding this comment.
This code should be moved into the last if block. It seems like error is not used outside this if block.
There was a problem hiding this comment.
Actually, error is set by CHECK_STATUS_PTHREAD (yes, silly macro...).
There was a problem hiding this comment.
Ow, ignored my previous comment in that case :-)
This is especially important if PyThread_acquire_lock() is called reentrantly
(for example from a signal handler).