I wrote: > Actually ... perhaps a better design would be to have > ConditionVariable[PrepareTo]Sleep auto-cancel any prepared sleep for > a different condition variable, analogously to what we just did in > ConditionVariableBroadcast, on the same theory that whenever control > returns to the other CV wait loop it can re-establish the relevant > state easily enough. I have to think that if the use of CVs grows > much, the existing restriction is going to become untenable anyway, > so why not just get rid of it?
Concretely, as per attached. regards, tom lane
diff --git a/src/backend/storage/lmgr/condition_variable.c b/src/backend/storage/lmgr/condition_variable.c index 0b9d676..b01864c 100644 --- a/src/backend/storage/lmgr/condition_variable.c +++ b/src/backend/storage/lmgr/condition_variable.c @@ -50,10 +50,6 @@ ConditionVariableInit(ConditionVariable *cv) * However, if the first test of the exit condition is likely to succeed, * it's more efficient to omit the ConditionVariablePrepareToSleep call. * See comments in ConditionVariableSleep for more detail. - * - * Only one condition variable can be used at a time, ie, - * ConditionVariableCancelSleep must be called before any attempt is made - * to sleep on a different condition variable. */ void ConditionVariablePrepareToSleep(ConditionVariable *cv) @@ -76,10 +72,14 @@ 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. + * 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; @@ -128,16 +128,16 @@ ConditionVariableSleep(ConditionVariable *cv, uint32 wait_event_info) * recommended because it avoids manipulations of the wait list, or not * met initially, in which case preparing first is better because it * avoids one extra test of the 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); - do { CHECK_FOR_INTERRUPTS(); diff --git a/src/include/storage/condition_variable.h b/src/include/storage/condition_variable.h index 7dac477..32e645c 100644 --- a/src/include/storage/condition_variable.h +++ b/src/include/storage/condition_variable.h @@ -40,9 +40,7 @@ extern void ConditionVariableInit(ConditionVariable *cv); * ConditionVariableSleep. Spurious wakeups are possible, but should be * infrequent. After exiting the loop, ConditionVariableCancelSleep must * be called to ensure that the process is no longer in the wait list for - * the condition variable. Only one condition variable can be used at a - * time, ie, ConditionVariableCancelSleep must be called before any attempt - * is made to sleep on a different condition variable. + * the condition variable. */ extern void ConditionVariableSleep(ConditionVariable *cv, uint32 wait_event_info); extern void ConditionVariableCancelSleep(void);