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;
    }