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);

Reply via email to