tevent: split out tevent_common_invoke_timer_handler()

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

We'll remove the _PRIVATE_ with the next release.

Signed-off-by: Stefan Metzmacher <metze@samba.org>

diff --git a/lib/tevent/tevent_internal.h b/lib/tevent/tevent_internal.h
index 48212949e16164b7321acb99a613e958afe8e261..bd1c2a0370a3b118e0a30be9605357efd8709702 100644 (file)
--- 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 ae1eb98d8b2a71227b5418fef7f98ad80af53350..d460e7001dcae0601915bcc867eacf375278e197 100644 (file)
--- 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,14 @@ static int tevent_common_timed_destructor(struct tevent_timer *te)
        }
        DLIST_REMOVE(te->event_ctx->timer_events, te);
 
-       return 0;
-}
+       te->event_ctx = NULL;
 
-static int tevent_common_timed_deny_destructor(struct tevent_timer *te)
-{
-       return -1;
+done:
+       if (te->busy) {
+               return -1;
+       }
+
+       return 0;
 }
 
 static void tevent_common_insert_timer(struct tevent_context *ev,
@@ -160,6 +168,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 +316,58 @@ void tevent_update_timer(struct tevent_timer *te, struct timeval next_event)
        tevent_common_insert_timer(ev, te, false);
 }
 
+_PRIVATE_
+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 +378,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 +410,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();
 }