BUG/MINOR: resolvers: check the SRV expiration date again under the lock

resolv_srvrq_expire_task() checks that the server's SRV record is expired
before taking the resolvers' lock and cleaning the server up. But the
resolvers' thread may assign an SRV record to this server between the two
(resolv_check_response() resets the task's expiration date under the
lock), in which case the server is then cleaned up and put down right
after being assigned a valid record, until the next SRV response.

This task is only scheduled when a server state file assigns an SRV
target to a server at boot, so the window is very narrow and only exists
once, and this was found by code review.

Let's check the expiration date again once the lock is held.

This patch may be backported to stable branches.

diff --git a/src/resolvers.c b/src/resolvers.c
index edb76a7c605c743bb190fb7429e126c1a8ec12a9..a9953fcf5b80ab2e827beab5febb61114027d0db 100644 (file)
--- a/src/resolvers.c
+++ b/src/resolvers.c
@@ -812,7 +812,11 @@ static struct task *resolv_srvrq_expire_task(struct task *t, void *context, unsi
 
        enter_resolver_code();
        HA_SPIN_LOCK(DNS_LOCK, &srv->srvrq->resolvers->lock);
-       resolv_srvrq_cleanup_srv(srv);
+       /* the server may have been assigned an SRV record by the resolvers
+        * in the mean time, which resets the expiration date.
+        */
+       if (tick_is_expired(t->expire, now_ms))
+               resolv_srvrq_cleanup_srv(srv);
        HA_SPIN_UNLOCK(DNS_LOCK, &srv->srvrq->resolvers->lock);
        leave_resolver_code();