Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 4af2190

Browse files
committed
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 aced5a9 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
1 parent 1776c81 commit 4af2190

File tree

1 file changed

+14
-8
lines changed

1 file changed

+14
-8
lines changed

src/backend/storage/lmgr/condition_variable.c

Lines changed: 14 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -70,10 +70,15 @@ ConditionVariablePrepareToSleep(ConditionVariable *cv)
7070
}
7171

7272
/*
73-
* It's not legal to prepare a sleep until the previous sleep has been
74-
* completed or canceled.
73+
* If some other sleep is already prepared, cancel it; this is necessary
74+
* because we have just one static variable tracking the prepared sleep,
75+
* and also only one cvWaitLink in our PGPROC. It's okay to do this
76+
* because whenever control does return to the other test-and-sleep loop,
77+
* its ConditionVariableSleep call will just re-establish that sleep as
78+
* the prepared one.
7579
*/
76-
Assert(cv_sleep_target == NULL);
80+
if (cv_sleep_target != NULL)
81+
ConditionVariableCancelSleep();
7782

7883
/* Record the condition variable on which we will sleep. */
7984
cv_sleep_target = cv;
@@ -121,16 +126,16 @@ ConditionVariableSleep(ConditionVariable *cv, uint32 wait_event_info)
121126
* allows you to skip manipulation of the wait list, or not met initially,
122127
* in which case preparing first allows you to skip a spurious test of the
123128
* caller's exit condition.
129+
*
130+
* If we are currently prepared to sleep on some other CV, we just cancel
131+
* that and prepare this one; see ConditionVariablePrepareToSleep.
124132
*/
125-
if (cv_sleep_target == NULL)
133+
if (cv_sleep_target != cv)
126134
{
127135
ConditionVariablePrepareToSleep(cv);
128136
return;
129137
}
130138

131-
/* Any earlier condition variable sleep must have been canceled. */
132-
Assert(cv_sleep_target == cv);
133-
134139
while (!done)
135140
{
136141
CHECK_FOR_INTERRUPTS();
@@ -246,7 +251,8 @@ ConditionVariableBroadcast(ConditionVariable *cv)
246251
* prepared CV sleep. The next call to ConditionVariableSleep will take
247252
* care of re-establishing the lost state.
248253
*/
249-
ConditionVariableCancelSleep();
254+
if (cv_sleep_target != NULL)
255+
ConditionVariableCancelSleep();
250256

251257
/*
252258
* Inspect the state of the queue. If it's empty, we have nothing to do.

0 commit comments

Comments
 (0)