From 4af2190eb04b6a9160f2820e00802eabc78b4b1c Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 9 Jan 2018 11:39:10 -0500 Subject: [PATCH] Allow ConditionVariable[PrepareTo]Sleep to auto-switch between CVs. The original coding here insisted that callers manually cancel any prepared sleep for one condition variable before starting a sleep on another one. While that's not a huge burden today, it seems like a gotcha that will bite us in future if the use of condition variables increases; anything we can do to make the use of this API simpler and more robust is attractive. Hence, allow these functions to automatically switch their attention to a different CV when required. This is safe for the same reason it was OK for commit aced5a92b to let a broadcast operation cancel any prepared CV sleep: whenever we return to the other test-and-sleep loop, we will automatically re-prepare that CV, paying at most an extra test of that loop's exit condition. Back-patch to v10 where condition variables were introduced. Ordinarily we would probably not back-patch a change like this, but since it does not invalidate any coding pattern that was legal before, it seems safe enough. Furthermore, there's an open bug in replorigin_drop() for which the simplest fix requires this. Even if we chose to fix that in some more complicated way, the hazard would remain that we might back-patch some other bug fix that requires this behavior. Patch by me, reviewed by Thomas Munro. Discussion: https://postgr.es/m/2437.1515368316@sss.pgh.pa.us --- src/backend/storage/lmgr/condition_variable.c | 22 ++++++++++++------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/src/backend/storage/lmgr/condition_variable.c b/src/backend/storage/lmgr/condition_variable.c index 552bdaaf80..730adf1c57 100644 --- a/src/backend/storage/lmgr/condition_variable.c +++ b/src/backend/storage/lmgr/condition_variable.c @@ -70,10 +70,15 @@ ConditionVariablePrepareToSleep(ConditionVariable *cv) } /* - * It's not legal to prepare a sleep until the previous sleep has been - * completed or canceled. + * If some other sleep is already prepared, cancel it; this is necessary + * because we have just one static variable tracking the prepared sleep, + * and also only one cvWaitLink in our PGPROC. It's okay to do this + * because whenever control does return to the other test-and-sleep loop, + * its ConditionVariableSleep call will just re-establish that sleep as + * the prepared one. */ - Assert(cv_sleep_target == NULL); + if (cv_sleep_target != NULL) + ConditionVariableCancelSleep(); /* Record the condition variable on which we will sleep. */ cv_sleep_target = cv; @@ -121,16 +126,16 @@ ConditionVariableSleep(ConditionVariable *cv, uint32 wait_event_info) * allows you to skip manipulation of the wait list, or not met initially, * in which case preparing first allows you to skip a spurious test of the * caller's exit condition. + * + * If we are currently prepared to sleep on some other CV, we just cancel + * that and prepare this one; see ConditionVariablePrepareToSleep. */ - if (cv_sleep_target == NULL) + if (cv_sleep_target != cv) { ConditionVariablePrepareToSleep(cv); return; } - /* Any earlier condition variable sleep must have been canceled. */ - Assert(cv_sleep_target == cv); - while (!done) { CHECK_FOR_INTERRUPTS(); @@ -246,7 +251,8 @@ ConditionVariableBroadcast(ConditionVariable *cv) * prepared CV sleep. The next call to ConditionVariableSleep will take * care of re-establishing the lost state. */ - ConditionVariableCancelSleep(); + if (cv_sleep_target != NULL) + ConditionVariableCancelSleep(); /* * Inspect the state of the queue. If it's empty, we have nothing to do. -- 2.40.0