At Mon, 2 Nov 2020 16:22:09 +1300, Thomas Munro <thomas.mu...@gmail.com> wrote
in
> On Mon, Nov 2, 2020 at 1:49 PM Kyotaro Horiguchi
> <horikyota....@gmail.com> wrote:
> > At Sat, 31 Oct 2020 11:40:53 -0300, Ranier Vilela <ranier...@gmail.com>
> > wrote in
> > > Per Coverity.
> > >
> > > If test set->latch against NULL, is why it can be NULL.
> > > ResetEvent can dereference NULL.
> >
> > If the returned event is WL_LATCH_SET, set->latch cannot be NULL. We
> > shouldn't inadvertently ignore the unexpected or broken situation.We
> > could put Assert instead, but I think that we don't need do something
> > here at all since SIGSEGV would be raised at the right location.
>
> Hmm. I changed that to support set->latch == NULL, so that you can
> use the long lived WES in the rare code paths that call WaitLatch()
> without a latch (for example the code I proposed at [1]). The Windows
Ooo. We don't update epoll events in that case. Ok, I understand
WL_LATCH_SET can fire while set->latch == NULL.
(I was confused by WaitEventAdjust* asserts set->latch != NULL for
WL_LATCH_SET. Shouldn't we move the check from ModifyWaitEvent() to
WaitEventAdjust*()?))
> version leaves the event handle of the most recently used latch in
> set->handles[n] (because AFAICS there is no way to have a "hole" in
> the handles array). The event can fire while you are waiting on "no
> latch". Perhaps it should be changed to
> ResetEvent(set->handles[cur_event->pos + 1])?
>
> [1]
> https://www.postgresql.org/message-id/flat/CA%2BhUKGK1607VmtrDUHQXrsooU%3Dap4g4R2yaoByWOOA3m8xevUQ%40mail.gmail.com
Seems right. Just doing that *seems* to work fine, but somehow I
cannot build on Windows for now...
regards.
--
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/src/backend/storage/ipc/latch.c b/src/backend/storage/ipc/latch.c
index 24d44c982d..318261962e 100644
--- a/src/backend/storage/ipc/latch.c
+++ b/src/backend/storage/ipc/latch.c
@@ -1835,7 +1835,11 @@ WaitEventSetWaitBlock(WaitEventSet *set, int cur_timeout,
if (cur_event->events == WL_LATCH_SET)
{
- if (!ResetEvent(set->latch->event))
+ /*
+ * We cannot use set->latch->event to reset the fired event if we
+ * actually aren't waiting on this latch now.
+ */
+ if (!ResetEvent(set->handles[cur_event->pos + 1]))
elog(ERROR, "ResetEvent failed: error code %lu", GetLastError());
if (set->latch && set->latch->is_set)
