]> git.ipfire.org Git - thirdparty/haproxy.git/commitdiff
MINOR: server: ensure max_events_at_once > 0 in server_atomic_sync()
authorAurelien DARRAGON <adarragon@haproxy.com>
Wed, 7 Aug 2024 16:04:08 +0000 (18:04 +0200)
committerAurelien DARRAGON <adarragon@haproxy.com>
Wed, 7 Aug 2024 16:31:35 +0000 (18:31 +0200)
In 8f1fd96 ("BUG/MEDIUM: server/addr: fix tune.events.max-events-at-once
event miss and leak"), we added a comment saying that
tune.events.max-events-at-once is assumed to be strictly positive.

It is so because the keyword parser forces values between 1 and 10000:
we don't want less than 1 because it wouldn't make any sense, and 10k
max because beyond that we could create contention in server_atomic_sync()

Now as the above commit implements a do..while it heavily relies on the
fact that the budget is at least 1. Upon soft-stop, we break away from
the loop without decrementing the budget. With all that in mind, it is
safe to assume that the 'remain' counter will only fall to 0 if the task
runs out of budget while doing work, in which case the task still exists
and must be rescheduled.

As seen in GH #2667 this assumption was ambiguous, so let's make it
official by adding a pair of BUG_ON() that make it explicit that it
works because remain 'cannot' be 0 unless the entire budget was
consumed.

No backport needed.

src/server.c

index 2f27df853878d22139e49f69a3762e9b348aabd1..c6f39012f538b98a2e5f5cc57d1cdf36a548b85e 100644 (file)
@@ -227,6 +227,8 @@ static struct task *server_atomic_sync(struct task *task, void *context, unsigne
        unsigned int remain = event_hdl_tune.max_events_at_once; // to limit max number of events per batch
        struct event_hdl_async_event *event;
 
+       BUG_ON(remain == 0); // event_hdl_tune.max_events_at_once is expected to be > 0
+
        /* check for new server events that we care about */
        do {
                event = event_hdl_async_equeue_pop(&server_atomic_sync_queue);
@@ -317,7 +319,7 @@ static struct task *server_atomic_sync(struct task *task, void *context, unsigne
                        srv_set_addr_desc(srv, 1);
                }
                event_hdl_async_free_event(event);
-       } while (--remain); // event_hdl_tune.max_events_at_once is expected to be > 0
+       } while (--remain);
 
        /* some events possibly required thread_isolation:
         * now that we are done, we must leave thread isolation before
@@ -334,6 +336,7 @@ static struct task *server_atomic_sync(struct task *task, void *context, unsigne
                 * Reschedule the task to finish where we left off if
                 * there are remaining events in the queue.
                 */
+               BUG_ON(task == NULL); // ending event doesn't decrement remain
                if (!event_hdl_async_equeue_isempty(&server_atomic_sync_queue))
                        task_wakeup(task, TASK_WOKEN_OTHER);
        }