Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitaced5a9

Browse files
committed
Rewrite ConditionVariableBroadcast() to avoid live-lock.
The original implementation of ConditionVariableBroadcast was, per itsself-description, "the dumbest way possible". Thomas Munro found outit was a bit too dumb. An awakened process may immediately re-queueitself, if the specific condition it's waiting for is not yet satisfied.If this happens before ConditionVariableBroadcast is able to see the waitqueue as empty, then ConditionVariableBroadcast will re-awaken the sameprocess, repeating the cycle. Given unlucky timing this back-and-forthcan repeat indefinitely; loops lasting thousands of seconds have beenseen in testing.To fix, add our own process to the end of the wait queue to serve as asentinel, and exit the broadcast loop once our process is not thereanymore. There are various special considerations described in thecomments, the principal disadvantage being that wakers can no longerbe sure whether they awakened a real waiter or just a sentinel. But inpractice nobody pays attention to the result of ConditionVariableSignalor ConditionVariableBroadcast anyway, so that problem seems hypothetical.Back-patch to v10 where condition_variable.c was introduced.Tom Lane and Thomas MunroDiscussion:https://postgr.es/m/CAEepm=0NWKehYw7NDoUSf8juuKOPRnCyY3vuaSvhrEWsOTAa3w@mail.gmail.com
1 parent19c47e7 commitaced5a9

File tree

1 file changed

+77
-5
lines changed

1 file changed

+77
-5
lines changed

‎src/backend/storage/lmgr/condition_variable.c

Lines changed: 77 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -214,15 +214,87 @@ int
214214
ConditionVariableBroadcast(ConditionVariable*cv)
215215
{
216216
intnwoken=0;
217+
intpgprocno=MyProc->pgprocno;
218+
PGPROC*proc=NULL;
219+
boolhave_sentinel= false;
220+
221+
/*
222+
* In some use-cases, it is common for awakened processes to immediately
223+
* re-queue themselves. If we just naively try to reduce the wakeup list
224+
* to empty, we'll get into a potentially-indefinite loop against such a
225+
* process. The semantics we really want are just to be sure that we have
226+
* wakened all processes that were in the list at entry. We can use our
227+
* own cvWaitLink as a sentinel to detect when we've finished.
228+
*
229+
* A seeming flaw in this approach is that someone else might signal the
230+
* CV and in doing so remove our sentinel entry. But that's fine: since
231+
* CV waiters are always added and removed in order, that must mean that
232+
* every previous waiter has been wakened, so we're done. We'll get an
233+
* extra "set" on our latch from the someone else's signal, which is
234+
* slightly inefficient but harmless.
235+
*
236+
* We can't insert our cvWaitLink as a sentinel if it's already in use in
237+
* some other proclist. While that's not expected to be true for typical
238+
* uses of this function, we can deal with it by simply canceling any
239+
* prepared CV sleep. The next call to ConditionVariableSleep will take
240+
* care of re-establishing the lost state.
241+
*/
242+
ConditionVariableCancelSleep();
217243

218244
/*
219-
* Let's just do this the dumbest way possible. We could try to dequeue
220-
* all the sleepers at once to save spinlock cycles, but it's a bit hard
221-
* to get that right in the face of possible sleep cancelations, and we
222-
* don't want to loop holding the mutex.
245+
* Inspect the state of the queue. If it's empty, we have nothing to do.
246+
* If there's exactly one entry, we need only remove and signal that
247+
* entry. Otherwise, remove the first entry and insert our sentinel.
223248
*/
224-
while (ConditionVariableSignal(cv))
249+
SpinLockAcquire(&cv->mutex);
250+
/* While we're here, let's assert we're not in the list. */
251+
Assert(!proclist_contains(&cv->wakeup,pgprocno,cvWaitLink));
252+
253+
if (!proclist_is_empty(&cv->wakeup))
254+
{
255+
proc=proclist_pop_head_node(&cv->wakeup,cvWaitLink);
256+
if (!proclist_is_empty(&cv->wakeup))
257+
{
258+
proclist_push_tail(&cv->wakeup,pgprocno,cvWaitLink);
259+
have_sentinel= true;
260+
}
261+
}
262+
SpinLockRelease(&cv->mutex);
263+
264+
/* Awaken first waiter, if there was one. */
265+
if (proc!=NULL)
266+
{
267+
SetLatch(&proc->procLatch);
225268
++nwoken;
269+
}
270+
271+
while (have_sentinel)
272+
{
273+
/*
274+
* Each time through the loop, remove the first wakeup list entry, and
275+
* signal it unless it's our sentinel. Repeat as long as the sentinel
276+
* remains in the list.
277+
*
278+
* Notice that if someone else removes our sentinel, we will waken one
279+
* additional process before exiting. That's intentional, because if
280+
* someone else signals the CV, they may be intending to waken some
281+
* third process that added itself to the list after we added the
282+
* sentinel. Better to give a spurious wakeup (which should be
283+
* harmless beyond wasting some cycles) than to lose a wakeup.
284+
*/
285+
proc=NULL;
286+
SpinLockAcquire(&cv->mutex);
287+
if (!proclist_is_empty(&cv->wakeup))
288+
proc=proclist_pop_head_node(&cv->wakeup,cvWaitLink);
289+
have_sentinel=proclist_contains(&cv->wakeup,pgprocno,cvWaitLink);
290+
SpinLockRelease(&cv->mutex);
291+
292+
if (proc!=NULL&&proc!=MyProc)
293+
{
294+
SetLatch(&proc->procLatch);
295+
++nwoken;
296+
}
297+
}
226298

227299
returnnwoken;
228300
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp