oauth: Fix postcondition for set_timer on macOS

On macOS, readding an EVFILT_TIMER to a kqueue does not appear to clear
out previously queued timer events, so checks for timer expiration do
not work correctly during token retrieval. Switching to IPv4-only
communication exposes the problem, because libcurl is no longer clearing
out other timeouts related to Happy Eyeballs dual-stack handling.

Fully remove and re-register the kqueue timer events during each call to
set_timer(), to clear out any stale expirations.

Author: Jacob Champion <jacob.champion@enterprisedb.com>
Discussion: https://postgr.es/m/CAOYmi%2Bn4EDOOUL27_OqYT2-F2rS6S%2B3mK-ppWb2Ec92UEoUbYA%40mail.gmail.com

diff --git a/src/interfaces/libpq/fe-auth-oauth-curl.c b/src/interfaces/libpq/fe-auth-oauth-curl.c
index 6e60a81574d1123ce76a3ed07234f99ca1dec47e..2d6d4b1a1234671da069fcfb8429157a0c44a2eb 100644 (file)
--- a/src/interfaces/libpq/fe-auth-oauth-curl.c
+++ b/src/interfaces/libpq/fe-auth-oauth-curl.c
@@ -1326,6 +1326,10 @@ register_socket(CURL *curl, curl_socket_t socket, int what, void *ctx,
  * in the set at all times and just disarm it when it's not needed. For kqueue,
  * the timer is removed completely when disabled to prevent stale timeouts from
  * remaining in the queue.
+ *
+ * To meet Curl requirements for the CURLMOPT_TIMERFUNCTION, implementations of
+ * set_timer must handle repeated calls by fully discarding any previous running
+ * or expired timer.
  */
 static bool
 set_timer(struct async_ctx *actx, long timeout)
@@ -1373,26 +1377,44 @@ set_timer(struct async_ctx *actx, long timeout)
                timeout = 1;
 #endif
 
-       /* Enable/disable the timer itself. */
-       EV_SET(&ev, 1, EVFILT_TIMER, timeout < 0 ? EV_DELETE : (EV_ADD | EV_ONESHOT),
-                  0, timeout, 0);
+       /*
+        * Always disable the timer, and remove it from the multiplexer, to clear
+        * out any already-queued events. (On some BSDs, adding an EVFILT_TIMER to
+        * a kqueue that already has one will clear stale events, but not on
+        * macOS.)
+        *
+        * If there was no previous timer set, the kevent calls will result in
+        * ENOENT, which is fine.
+        */
+       EV_SET(&ev, 1, EVFILT_TIMER, EV_DELETE, 0, 0, 0);
        if (kevent(actx->timerfd, &ev, 1, NULL, 0, NULL) < 0 && errno != ENOENT)
        {
-               actx_error(actx, "setting kqueue timer to %ld: %m", timeout);
+               actx_error(actx, "deleting kqueue timer: %m", timeout);
                return false;
        }
 
-       /*
-        * Add/remove the timer to/from the mux. (In contrast with epoll, if we
-        * allowed the timer to remain registered here after being disabled, the
-        * mux queue would retain any previous stale timeout notifications and
-        * remain readable.)
-        */
-       EV_SET(&ev, actx->timerfd, EVFILT_READ, timeout < 0 ? EV_DELETE : EV_ADD,
-                  0, 0, 0);
+       EV_SET(&ev, actx->timerfd, EVFILT_READ, EV_DELETE, 0, 0, 0);
        if (kevent(actx->mux, &ev, 1, NULL, 0, NULL) < 0 && errno != ENOENT)
        {
-               actx_error(actx, "could not update timer on kqueue: %m");
+               actx_error(actx, "removing kqueue timer from multiplexer: %m");
+               return false;
+       }
+
+       /* If we're not adding a timer, we're done. */
+       if (timeout < 0)
+               return true;
+
+       EV_SET(&ev, 1, EVFILT_TIMER, (EV_ADD | EV_ONESHOT), 0, timeout, 0);
+       if (kevent(actx->timerfd, &ev, 1, NULL, 0, NULL) < 0)
+       {
+               actx_error(actx, "setting kqueue timer to %ld: %m", timeout);
+               return false;
+       }
+
+       EV_SET(&ev, actx->timerfd, EVFILT_READ, EV_ADD, 0, 0, 0);
+       if (kevent(actx->mux, &ev, 1, NULL, 0, NULL) < 0)
+       {
+               actx_error(actx, "adding kqueue timer to multiplexer: %m");
                return false;
        }