From: Alan T. DeKok <aland@freeradius.org>
Date: Wed, 18 Dec 2019 15:49:42 +0000 (-0500)
Subject: add "to do" list for timers, too
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5874c88df33da80db2f66a6abab47d12e0355866;p=thirdparty%2Ffreeradius-server.git

add "to do" list for timers, too

New timers were added while running timers. Instead of
being added to the main heap, they were added to the
"in progress" list.  We can now add them to the main
heap.

Doing it this way prevents the server from running
into an infinite loop when it takes a long time to
service FD events.  The timer callback MAY add a new
timer which is in the past.  So the above loop could
run forever.
---

diff --git a/src/lib/util/event.c b/src/lib/util/event.c
index 6371a7e2b9d..fec6258e629 100644
--- a/src/lib/util/event.c
+++ b/src/lib/util/event.c
@@ -83,12 +83,13 @@ static size_t kevent_filter_table_len = NUM_ELEMENTS(kevent_filter_table);
 struct fr_event_timer {
 	fr_event_list_t		*el;			//!< because talloc_parent() is O(N) in number of objects
 	fr_time_t		when;			//!< When this timer should fire.
-	fr_event_timer_cb_t		callback;		//!< Callback to execute when the timer fires.
+	fr_event_timer_cb_t	callback;		//!< Callback to execute when the timer fires.
 	void const		*uctx;			//!< Context pointer to pass to the callback.
 	TALLOC_CTX		*linked_ctx;		//!< talloc ctx this event was bound to.
 
 	fr_event_timer_t const	**parent;		//!< Previous timer.
 	int32_t			heap_id;	       	//!< Where to store opaque heap data.
+	fr_event_timer_t	*next;			//!< in linked list of event timers
 };
 
 typedef enum {
@@ -321,6 +322,7 @@ struct fr_event_list {
 							///< handlers complete.
 
 	fr_event_fd_t		*fd_to_free;		//!< File descriptor events pending deletion.
+	fr_event_timer_t	*ev_to_add;		//!< event to add
 };
 
 /** Compare two timer events to see which one should occur first
@@ -1104,7 +1106,11 @@ int fr_event_timer_at(TALLOC_CTX *ctx, fr_event_list_t *el, fr_event_timer_t con
 	ev->linked_ctx = ctx;
 	ev->parent = ev_p;
 
-	if (unlikely(fr_heap_insert(el->times, ev) < 0)) {
+	if (el->in_handler) {
+		ev->next = el->ev_to_add;
+		el->ev_to_add = ev;
+
+	} else if (unlikely(fr_heap_insert(el->times, ev) < 0)) {
 		talloc_free(ev);
 		return -1;
 	}
@@ -1710,7 +1716,6 @@ service:
 			break;
 		}
 	}
-	el->in_handler = false;
 
 	/*
 	 *	Process any deferred frees performed
@@ -1724,7 +1729,8 @@ service:
 	talloc_list_free(&el->fd_to_free);
 
 	/*
-	 *	Run all of the timer events.
+	 *	Run all of the timer events.  Note that these can add
+	 *	new timers!
 	 */
 	if (fr_heap_num_elements(el->times) > 0) {
 		do {
@@ -1732,6 +1738,34 @@ service:
 		} while (fr_event_timer_run(el, &when) == 1);
 	}
 
+	/*
+	 *	New timers were added while running timers. Instead of
+	 *	being added to the main heap, they were added to the
+	 *	"in progress" list.  We can now add them to the main
+	 *	heap.
+	 *
+	 *	Doing it this way prevents the server from running
+	 *	into an infinite loop when it takes a long time to
+	 *	service FD events.  The timer callback MAY add a new
+	 *	timer which is in the past.  So the above loop could
+	 *	run forever.
+	 */
+	if (el->ev_to_add) {
+		fr_event_timer_t *ev, *next;
+
+		while ((ev = el->ev_to_add) != NULL) {
+			next = ev->next;
+			ev->next = NULL;
+
+			if (unlikely(fr_heap_insert(el->times, ev) < 0)) {
+				talloc_free(ev);
+			}
+			el->ev_to_add = next;
+		}
+	}
+
+	el->in_handler = false;
+
 	/*
 	 *	Run all of the post-processing events.
 	 */