From 8171af9697470138b936ab0870f3e5dacd306a00 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Tue, 24 Dec 2019 14:36:16 +1300 Subject: [PATCH] Don't reset latch in ConditionVariablePrepareToSleep(). It's not OK to do that without calling CHECK_FOR_INTERRUPTS(), and it's probably better to make all latch interaction code follow the standard pattern. Let's leave the latch set if it's set, so that the next wait loop (mostly likely ConditionVariableSleep()) sees it and handles it, if we don't reach some other CHECK_FOR_INTERRUPTS() first. One consequence of this bug was that a SIGTERM delivered in a very narrow timing window could leave a parallel worker process waiting forever for a condition variable that will never be signaled, after an error was raised in other process. Back-patch to 10, where condition variables were introduced. The bug was probably not easy to hit before commit 1321509f in master, but it was still a violation of the latch/interrupt protocol, so repair. In the back-branches only, also move the CHECK_FOR_INTERRUPTS() to after the ResetLatch() call, to close a related race: a SIGTERM that arrives around that same time as a condition variable signal could be merged, and the ConditionVariableSleep() could return having reset the latch and noticed the CV signal, and then the caller could wait on a latch. Author: Thomas Munro Reviewed-by: Shawn Debnath Reported-by: Tomas Vondra Discussion: https://postgr.es/m/CA%2BhUKGJOm8zZHjVA8svoNT3tHY0XdqmaC_kHitmgXDQM49m1dA%40mail.gmail.com --- src/backend/storage/lmgr/condition_variable.c | 6 ------ 1 file changed, 6 deletions(-) diff --git a/src/backend/storage/lmgr/condition_variable.c b/src/backend/storage/lmgr/condition_variable.c index e08507f0cc..663db95aa8 100644 --- a/src/backend/storage/lmgr/condition_variable.c +++ b/src/backend/storage/lmgr/condition_variable.c @@ -93,12 +93,6 @@ ConditionVariablePrepareToSleep(ConditionVariable *cv) /* Record the condition variable on which we will sleep. */ cv_sleep_target = cv; - /* - * Reset my latch before adding myself to the queue, to ensure that we - * don't miss a wakeup that occurs immediately. - */ - ResetLatch(MyLatch); - /* Add myself to the wait queue. */ SpinLockAcquire(&cv->mutex); proclist_push_tail(&cv->wakeup, pgprocno, cvWaitLink); -- 2.23.0