From 410dce1670a04de81d533dd1b5456e363b144ca8 Mon Sep 17 00:00:00 2001 From: Jacob Champion Date: Thu, 6 Mar 2025 15:02:37 -0800 Subject: [PATCH 2/5] oauth: Fix postcondition for set_timer on BSD 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. Discussion: https://postgr.es/m/CAOYmi%2Bn4EDOOUL27_OqYT2-F2rS6S%2B3mK-ppWb2Ec92UEoUbYA%40mail.gmail.com --- src/interfaces/libpq/fe-auth-oauth-curl.c | 48 +++++++++++++++++------ 1 file 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; } -- 2.34.1