efl/timer: correctly handle recursion for timer processing
authorMike Blumenkrantz <zmike@samsung.com>
Thu, 24 Oct 2019 15:22:49 +0000 (11:22 -0400)
committerWonki Kim <wonki_.kim@samsung.com>
Mon, 11 Nov 2019 02:20:39 +0000 (11:20 +0900)
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 <cedric.bail@free.fr>
Differential Revision: https://phab.enlightenment.org/D10515

src/lib/ecore/ecore_timer.c

index ed6d7c3..518d06c 100644 (file)
@@ -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);
      }