From 8f3d33d5e854bd963ec72ff06b4f3b9302ff2b4b Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Sat, 8 Aug 2020 07:52:47 +1200 Subject: [PATCH 3/3] Remove self-pipe in WAIT_USE_KQUEUE builds. Instead of using a signal handler for latch wakeup, use EVFILT_SIGNAL. This closes the race condition traditionally solved by the self-pipe trick. --- src/backend/storage/ipc/latch.c | 41 +++++++++++++++++++++++++-------- 1 file changed, 32 insertions(+), 9 deletions(-) diff --git a/src/backend/storage/ipc/latch.c b/src/backend/storage/ipc/latch.c index 0cb5875228..ae3d904791 100644 --- a/src/backend/storage/ipc/latch.c +++ b/src/backend/storage/ipc/latch.c @@ -24,6 +24,9 @@ * Since epoll_pwait() atomically unblocks and begins waiting, there is no * race, so there is no need for the self-pipe trick. * + * The kqueue() implementation uses yet another technique: it uses SIGURG, + * but installs no signal handler, and waits for signals using EVFILT_SIGNAL. + * * The Windows implementation uses Windows events that are inherited by all * postmaster child processes. There's no need for the self-pipe trick there. * @@ -85,7 +88,7 @@ #error "no wait set implementation available" #endif -#if defined(WAIT_USE_POLL) || defined(WAIT_USE_KQUEUE) +#if defined(WAIT_USE_POLL) #define NEED_SELF_PIPE_TRICK /* Use a signal that is overloaded with the procsignal mechanism. */ #define LATCH_SIGNAL SIGUSR1 @@ -267,6 +270,9 @@ InitializeLatchSupport(void) #ifdef WAIT_USE_EPOLL pqsignal(SIGURG, sigurg_handler); #endif +#ifdef WAIT_USE_KQUEUE + pqsignal(SIGURG, SIG_IGN); +#endif } void @@ -888,7 +894,9 @@ AddWaitEventToSet(WaitEventSet *set, uint32 events, pgsocket fd, Latch *latch, event->fd = selfpipe_readfd; #else event->fd = PGINVALID_SOCKET; +#ifdef WAIT_USE_EPOLL return event->pos; +#endif #endif } else if (events == WL_POSTMASTER_DEATH) @@ -1096,6 +1104,18 @@ WaitEventAdjustKqueueAddPostmaster(struct kevent *k_ev, WaitEvent *event) AccessWaitEvent(k_ev) = event; } +static inline void +WaitEventAdjustKqueueAddLatch(struct kevent *k_ev, WaitEvent *event) +{ + /* For now latch can only be added, not removed. */ + k_ev->ident = SIGURG; + k_ev->filter = EVFILT_SIGNAL; + k_ev->flags = EV_ADD; + k_ev->fflags = 0; + k_ev->data = 0; + AccessWaitEvent(k_ev) = event; +} + /* * old_events is the previous event mask, used to compute what has changed. */ @@ -1127,6 +1147,14 @@ WaitEventAdjustKqueue(WaitEventSet *set, WaitEvent *event, int old_events) */ WaitEventAdjustKqueueAddPostmaster(&k_ev[count++], event); } + else if (event->events == WL_LATCH_SET) + { + /* + * Unlike all the other implementations, we detect latch wakeup using + * signal events, rather than installing a signal handler. + */ + WaitEventAdjustKqueueAddLatch(&k_ev[count++], event); + } else { /* @@ -1134,11 +1162,9 @@ WaitEventAdjustKqueue(WaitEventSet *set, WaitEvent *event, int old_events) * old event mask to the new event mask, since kevent treats readable * and writable as separate events. */ - if (old_events == WL_LATCH_SET || - (old_events & WL_SOCKET_READABLE)) + if (old_events & WL_SOCKET_READABLE) old_filt_read = true; - if (event->events == WL_LATCH_SET || - (event->events & WL_SOCKET_READABLE)) + if (event->events & WL_SOCKET_READABLE) new_filt_read = true; if (old_events & WL_SOCKET_WRITEABLE) old_filt_write = true; @@ -1591,11 +1617,8 @@ WaitEventSetWaitBlock(WaitEventSet *set, int cur_timeout, occurred_events->events = 0; if (cur_event->events == WL_LATCH_SET && - cur_kqueue_event->filter == EVFILT_READ) + cur_kqueue_event->filter == EVFILT_SIGNAL) { - /* There's data in the self-pipe, clear it. */ - drainSelfPipe(); - if (set->latch->is_set) { occurred_events->fd = PGINVALID_SOCKET; -- 2.20.1