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

1 files changed, 35 insertions, 13 deletions

diff --git a/src/interfaces/libpq/fe-auth-oauth-curl.c b/src/interfaces/libpq/fe-auth-oauth-curl.c
index 6e60a81574d..2d6d4b1a123 100644
--- 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;
 	}