Remove return values of ConditionVariableSignal/Broadcast.
In the wake of commit aced5a92b, the semantics of these results are a bit squishy: we can tell whether we signaled some other process(es), but we do not know which ones were real waiters versus mere sentinels for ConditionVariableBroadcast operations. It does not help much that ConditionVariableBroadcast will attempt to pass on the signal to the next real waiter, because (a) there might not be one, and (b) that will only happen awhile later, anyway. So these results could overstate how much effect the calls really had. However, no existing caller of either function pays any attention to its result value, so it seems reasonable to just define that as a required property of a correct algorithm. To encourage correctness and save some tiny number of cycles, change both functions to return void. Patch by me, per an observation by Thomas Munro. No back-patch, since if any third parties happen to be using these functions, they might not appreciate an API break in a minor release. Discussion: https://postgr.es/m/CAEepm=0NWKehYw7NDoUSf8juuKOPRnCyY3vuaSvhrEWsOTAa3w@mail.gmail.com
This commit is contained in:
parent
3cac0ec859
commit
ccf312a448
src
@ -186,11 +186,14 @@ ConditionVariableCancelSleep(void)
|
||||
}
|
||||
|
||||
/*
|
||||
* Wake up one sleeping process, assuming there is at least one.
|
||||
* Wake up the oldest process sleeping on the CV, if there is any.
|
||||
*
|
||||
* The return value indicates whether or not we woke somebody up.
|
||||
* Note: it's difficult to tell whether this has any real effect: we know
|
||||
* whether we took an entry off the list, but the entry might only be a
|
||||
* sentinel. Hence, think twice before proposing that this should return
|
||||
* a flag telling whether it woke somebody.
|
||||
*/
|
||||
bool
|
||||
void
|
||||
ConditionVariableSignal(ConditionVariable *cv)
|
||||
{
|
||||
PGPROC *proc = NULL;
|
||||
@ -203,24 +206,19 @@ ConditionVariableSignal(ConditionVariable *cv)
|
||||
|
||||
/* If we found someone sleeping, set their latch to wake them up. */
|
||||
if (proc != NULL)
|
||||
{
|
||||
SetLatch(&proc->procLatch);
|
||||
return true;
|
||||
}
|
||||
|
||||
/* No sleeping processes. */
|
||||
return false;
|
||||
}
|
||||
|
||||
/*
|
||||
* Wake up all sleeping processes.
|
||||
* Wake up all processes sleeping on the given CV.
|
||||
*
|
||||
* The return value indicates the number of processes we woke.
|
||||
* This guarantees to wake all processes that were sleeping on the CV
|
||||
* at time of call, but processes that add themselves to the list mid-call
|
||||
* will typically not get awakened.
|
||||
*/
|
||||
int
|
||||
void
|
||||
ConditionVariableBroadcast(ConditionVariable *cv)
|
||||
{
|
||||
int nwoken = 0;
|
||||
int pgprocno = MyProc->pgprocno;
|
||||
PGPROC *proc = NULL;
|
||||
bool have_sentinel = false;
|
||||
@ -270,10 +268,7 @@ ConditionVariableBroadcast(ConditionVariable *cv)
|
||||
|
||||
/* Awaken first waiter, if there was one. */
|
||||
if (proc != NULL)
|
||||
{
|
||||
SetLatch(&proc->procLatch);
|
||||
++nwoken;
|
||||
}
|
||||
|
||||
while (have_sentinel)
|
||||
{
|
||||
@ -297,11 +292,6 @@ ConditionVariableBroadcast(ConditionVariable *cv)
|
||||
SpinLockRelease(&cv->mutex);
|
||||
|
||||
if (proc != NULL && proc != MyProc)
|
||||
{
|
||||
SetLatch(&proc->procLatch);
|
||||
++nwoken;
|
||||
}
|
||||
}
|
||||
|
||||
return nwoken;
|
||||
}
|
||||
|
@ -53,7 +53,7 @@ extern void ConditionVariableCancelSleep(void);
|
||||
extern void ConditionVariablePrepareToSleep(ConditionVariable *);
|
||||
|
||||
/* Wake up a single waiter (via signal) or all waiters (via broadcast). */
|
||||
extern bool ConditionVariableSignal(ConditionVariable *);
|
||||
extern int ConditionVariableBroadcast(ConditionVariable *);
|
||||
extern void ConditionVariableSignal(ConditionVariable *cv);
|
||||
extern void ConditionVariableBroadcast(ConditionVariable *cv);
|
||||
|
||||
#endif /* CONDITION_VARIABLE_H */
|
||||
|
Loading…
x
Reference in New Issue
Block a user