tevent: split out tevent_common_invoke_timer_handler()

As side effect this avoids tricks with an extra
tevent_common_timed_deny_destructor().

We'll undo the 0.9.36 ABI change on the 0.9.37 release
at the end of this patchset.

Signed-off-by: Stefan Metzmacher <metze@samba.org>
Reviewed-by: Ralph Boehme <slow@samba.org>

3 files changed, 78 insertions, 38 deletions

diff --git a/lib/tevent/ABI/tevent-0.9.36.sigs b/lib/tevent/ABI/tevent-0.9.36.sigs
index 230f6b9f26b..bb89cc72d1e 100644
--- a/lib/tevent/ABI/tevent-0.9.36.sigs
+++ b/lib/tevent/ABI/tevent-0.9.36.sigs
@@ -33,6 +33,7 @@ tevent_common_fd_set_close_fn: void (struct tevent_fd *, tevent_fd_close_fn_t)
 tevent_common_fd_set_flags: void (struct tevent_fd *, uint16_t)
 tevent_common_have_events: bool (struct tevent_context *)
 tevent_common_invoke_signal_handler: int (struct tevent_signal *, int, int, void *, bool *)
+tevent_common_invoke_timer_handler: int (struct tevent_timer *, struct timeval, bool *)
 tevent_common_loop_immediate: bool (struct tevent_context *)
 tevent_common_loop_timer_delay: struct timeval (struct tevent_context *)
 tevent_common_loop_wait: int (struct tevent_context *, const char *)
diff --git a/lib/tevent/tevent_internal.h b/lib/tevent/tevent_internal.h
index f36cd219f68..8126414d683 100644
--- a/lib/tevent/tevent_internal.h
+++ b/lib/tevent/tevent_internal.h
@@ -187,6 +187,8 @@ struct tevent_fd {
 struct tevent_timer {
 	struct tevent_timer *prev, *next;
 	struct tevent_context *event_ctx;
+	bool busy;
+	bool destroyed;
 	struct timeval next_event;
 	tevent_timer_handler_t handler;
 	/* this is private for the specific handler */
@@ -355,6 +357,9 @@ struct tevent_timer *tevent_common_add_timer_v2(struct tevent_context *ev,
 					        const char *handler_name,
 					        const char *location);
 struct timeval tevent_common_loop_timer_delay(struct tevent_context *);
+int tevent_common_invoke_timer_handler(struct tevent_timer *te,
+				       struct timeval current_time,
+				       bool *removed);
 
 void tevent_common_schedule_immediate(struct tevent_immediate *im,
 				      struct tevent_context *ev,
diff --git a/lib/tevent/tevent_timed.c b/lib/tevent/tevent_timed.c
index ae1eb98d8b2..cb948d4ba29 100644
--- a/lib/tevent/tevent_timed.c
+++ b/lib/tevent/tevent_timed.c
@@ -133,6 +133,12 @@ struct timeval tevent_timeval_current_ofs(uint32_t secs, uint32_t usecs)
 */
 static int tevent_common_timed_destructor(struct tevent_timer *te)
 {
+	if (te->destroyed) {
+		tevent_common_check_double_free(te, "tevent_timer double free");
+		goto done;
+	}
+	te->destroyed = true;
+
 	if (te->event_ctx == NULL) {
 		return 0;
 	}
@@ -146,12 +152,13 @@ static int tevent_common_timed_destructor(struct tevent_timer *te)
 	}
 	DLIST_REMOVE(te->event_ctx->timer_events, te);
 
-	return 0;
-}
+	te->event_ctx = NULL;
+done:
+	if (te->busy) {
+		return -1;
+	}
 
-static int tevent_common_timed_deny_destructor(struct tevent_timer *te)
-{
-	return -1;
+	return 0;
 }
 
 static void tevent_common_insert_timer(struct tevent_context *ev,
@@ -160,6 +167,11 @@ static void tevent_common_insert_timer(struct tevent_context *ev,
 {
 	struct tevent_timer *prev_te = NULL;
 
+	if (te->destroyed) {
+		tevent_abort(ev, "tevent_timer use after free");
+		return;
+	}
+
 	/* keep the list ordered */
 	if (optimize_zero && tevent_timeval_is_zero(&te->next_event)) {
 		/*
@@ -303,6 +315,57 @@ void tevent_update_timer(struct tevent_timer *te, struct timeval next_event)
 	tevent_common_insert_timer(ev, te, false);
 }
 
+int tevent_common_invoke_timer_handler(struct tevent_timer *te,
+				       struct timeval current_time,
+				       bool *removed)
+{
+	if (removed != NULL) {
+		*removed = false;
+	}
+
+	if (te->event_ctx == NULL) {
+		return 0;
+	}
+
+	/*
+	 * We need to remove the timer from the list before calling the
+	 * handler because in a semi-async inner event loop called from the
+	 * handler we don't want to come across this event again -- vl
+	 */
+	if (te->event_ctx->last_zero_timer == te) {
+		te->event_ctx->last_zero_timer = DLIST_PREV(te);
+	}
+	DLIST_REMOVE(te->event_ctx->timer_events, te);
+
+	tevent_debug(te->event_ctx, TEVENT_DEBUG_TRACE,
+		     "Running timer event %p \"%s\"\n",
+		     te, te->handler_name);
+
+	/*
+	 * If the timed event was registered for a zero current_time,
+	 * then we pass a zero timeval here too! To avoid the
+	 * overhead of gettimeofday() calls.
+	 *
+	 * otherwise we pass the current time
+	 */
+	te->busy = true;
+	te->handler(te->event_ctx, te, current_time, te->private_data);
+	te->busy = false;
+
+	tevent_debug(te->event_ctx, TEVENT_DEBUG_TRACE,
+		     "Ending timer event %p \"%s\"\n",
+		     te, te->handler_name);
+
+	te->event_ctx = NULL;
+	talloc_set_destructor(te, NULL);
+	TALLOC_FREE(te);
+
+	if (removed != NULL) {
+		*removed = true;
+	}
+
+	return 0;
+}
 /*
   do a single event loop using the events defined in ev
 
@@ -313,6 +376,7 @@ struct timeval tevent_common_loop_timer_delay(struct tevent_context *ev)
 {
 	struct timeval current_time = tevent_timeval_zero();
 	struct tevent_timer *te = ev->timer_events;
+	int ret;
 
 	if (!te) {
 		/* have a default tick time of 30 seconds. This guarantees
@@ -344,40 +408,10 @@ struct timeval tevent_common_loop_timer_delay(struct tevent_context *ev)
 	/*
 	 * ok, we have a timed event that we'll process ...
 	 */
-
-	/* deny the handler to free the event */
-	talloc_set_destructor(te, tevent_common_timed_deny_destructor);
-
-	/* We need to remove the timer from the list before calling the
-	 * handler because in a semi-async inner event loop called from the
-	 * handler we don't want to come across this event again -- vl */
-	if (ev->last_zero_timer == te) {
-		ev->last_zero_timer = DLIST_PREV(te);
+	ret = tevent_common_invoke_timer_handler(te, current_time, NULL);
+	if (ret != 0) {
+		tevent_abort(ev, "tevent_common_invoke_timer_handler() failed");
 	}
-	DLIST_REMOVE(ev->timer_events, te);
-
-	tevent_debug(te->event_ctx, TEVENT_DEBUG_TRACE,
-		     "Running timer event %p \"%s\"\n",
-		     te, te->handler_name);
-
-	/*
-	 * If the timed event was registered for a zero current_time,
-	 * then we pass a zero timeval here too! To avoid the
-	 * overhead of gettimeofday() calls.
-	 *
-	 * otherwise we pass the current time
-	 */
-	te->handler(ev, te, current_time, te->private_data);
-
-	/* The destructor isn't necessary anymore, we've already removed the
-	 * event from the list. */
-	talloc_set_destructor(te, NULL);
-
-	tevent_debug(te->event_ctx, TEVENT_DEBUG_TRACE,
-		     "Ending timer event %p \"%s\"\n",
-		     te, te->handler_name);
-
-	talloc_free(te);
 
 	return tevent_timeval_zero();
 }