From dcc7467c0031b632a06d1667e9effc5be71d8a94 Mon Sep 17 00:00:00 2001 From: Mike Blumenkrantz Date: Thu, 24 Oct 2019 11:22:49 -0400 Subject: [PATCH] efl/timer: correctly handle recursion for timer processing if the currently-processed timer is recursively deleted (efl_del) while it is inside the timer tick event callback, we must correctly handle this case: * in the place where a timer's inlist is de-linked, we must check to see if the timer is the current timer and then update that pointer with the next timer in the list * in the post-tick part of timer processing, we must NOT update the current timer pointer if we detect that it has been updated recursively this fixes processing of timers in the mainloop to trigger more than one legacy timer per mainloop iteration and likely has some (positive) impact on mainloop throughput @fix Reviewed-by: Cedric BAIL Differential Revision: https://phab.enlightenment.org/D10515 --- src/lib/ecore/ecore_timer.c | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/lib/ecore/ecore_timer.c b/src/lib/ecore/ecore_timer.c index ed6d7c340d..518d06c06a 100644 --- a/src/lib/ecore/ecore_timer.c +++ b/src/lib/ecore/ecore_timer.c @@ -472,8 +472,12 @@ _efl_loop_timer_efl_object_parent_set(Eo *obj, Efl_Loop_Timer_Data *pd, Efl_Obje // Remove the timer from all possible pending list first = eina_inlist_first(EINA_INLIST_GET(pd)); if (first == pd->loop_data->timers) - pd->loop_data->timers = eina_inlist_remove - (pd->loop_data->timers, EINA_INLIST_GET(pd)); + { + /* if this timer is currently being processed, update the pointer here so it is not lost */ + if (pd == pd->loop_data->timer_current) + pd->loop_data->timer_current = (Efl_Loop_Timer_Data*)EINA_INLIST_GET(pd)->next; + pd->loop_data->timers = eina_inlist_remove(pd->loop_data->timers, EINA_INLIST_GET(pd)); + } else if (first == pd->loop_data->suspended) pd->loop_data->suspended = eina_inlist_remove (pd->loop_data->suspended, EINA_INLIST_GET(pd)); @@ -642,14 +646,24 @@ _efl_loop_timer_expired_call(Eo *obj EINA_UNUSED, Efl_Loop_Data *pd, double when efl_ref(timer->object); eina_evlog("+timer", timer, 0.0, NULL); + /* this can remove timer from its inlist in the legacy codepath */ efl_event_callback_call(timer->object, EFL_LOOP_TIMER_EVENT_TIMER_TICK, NULL); eina_evlog("-timer", timer, 0.0, NULL); // may have changed in recursive main loops // this current timer can not die yet as we hold a reference on it + /* this is tricky: the current timer cannot be deleted, but it CAN be removed from its inlist, + * thus breaking timer processing + */ if (pd->timer_current) - pd->timer_current = (Efl_Loop_Timer_Data *) - EINA_INLIST_GET(pd->timer_current)->next; + { + if (pd->timer_current == timer) + pd->timer_current = (Efl_Loop_Timer_Data *)EINA_INLIST_GET(pd->timer_current)->next; + /* assume this has otherwise been modified either due to recursive mainloop processing or + * the timer being removed from its inlist and carefully updating pd->timer_current in the + * process as only the most elite of engineers would think to do + */ + } _efl_loop_timer_reschedule(timer, when); efl_unref(timer->object); }