ecore/timer: correctly handle recursive deletion of legacy timers

if a legacy timer callback returns false, the timer is deleted. in the
case where the legacy timer is deleted inside the callback while the same
timer is ticking recursively, however, the deletion must be deferred until
the outer-most frame of the timer's callstack has returned from the callback
in order to avoid improperly handling the timer

@fix

Reviewed-by: Cedric BAIL <cedric.bail@free.fr>
Differential Revision: https://phab.enlightenment.org/D10545

1 files changed, 27 insertions, 15 deletions

diff --git a/src/lib/ecore/ecore_timer.c b/src/lib/ecore/ecore_timer.c
index 518d06c06a..e07aa9f666 100644
--- a/src/lib/ecore/ecore_timer.c
+++ b/src/lib/ecore/ecore_timer.c
@@ -24,7 +24,7 @@ struct _Ecore_Timer_Legacy
    Eina_Bool inside_call : 1;
    Eina_Bool delete_me   : 1;
 };
-
+typedef struct _Ecore_Timer_Legacy Ecore_Timer_Legacy;
 struct _Efl_Loop_Timer_Data
 {
    EINA_INLIST;
@@ -32,6 +32,7 @@ struct _Efl_Loop_Timer_Data
    Eo            *object;
    Eo            *loop;
    Efl_Loop_Data *loop_data;
+   Ecore_Timer_Legacy *legacy;
 
    double     in;
    double     at;
@@ -47,8 +48,6 @@ struct _Efl_Loop_Timer_Data
    Eina_Bool  finalized   : 1;
 };
 
-typedef struct _Ecore_Timer_Legacy Ecore_Timer_Legacy;
-
 static void _efl_loop_timer_util_delay(Efl_Loop_Timer_Data *timer, double add);
 static void _efl_loop_timer_util_instanciate(Efl_Loop_Data *loop, Efl_Loop_Timer_Data *timer);
 static void _efl_loop_timer_set(Efl_Loop_Timer_Data *timer, double at, double in);
@@ -156,12 +155,19 @@ static void
 _ecore_timer_legacy_tick(void *data, const Efl_Event *event)
 {
    Ecore_Timer_Legacy *legacy = data;
+   Eina_Bool inside_call = legacy->inside_call;
 
    legacy->inside_call = 1;
-   if (!_ecore_call_task_cb(legacy->func, (void *)legacy->data) ||
-       legacy->delete_me)
-     efl_del(event->object);
-   else legacy->inside_call = 0;
+   if (!_ecore_call_task_cb(legacy->func, (void *)legacy->data) || legacy->delete_me)
+     {
+        legacy->delete_me = EINA_TRUE;
+        /* cannot destroy timer if recursing */
+        if (!inside_call)
+          efl_del(event->object);
+     }
+   /* only unset flag if not currently recursing */
+   else if (!inside_call)
+     legacy->inside_call = 0;
 }
 
 EFL_CALLBACKS_ARRAY_DEFINE(legacy_timer,
@@ -172,6 +178,7 @@ EAPI Ecore_Timer *
 ecore_timer_add(double in, Ecore_Task_Cb func, const void *data)
 {
    Ecore_Timer_Legacy *legacy;
+   Efl_Loop_Timer_Data *td;
    Eo *timer;
 
    EINA_MAIN_LOOP_CHECK_RETURN_VAL(NULL);
@@ -186,8 +193,9 @@ ecore_timer_add(double in, Ecore_Task_Cb func, const void *data)
    legacy->data = data;
    timer = efl_add(MY_CLASS, efl_main_loop_get(),
                   efl_event_callback_array_add(efl_added, legacy_timer(), legacy),
-                  efl_key_data_set(efl_added, "_legacy", legacy),
                   efl_loop_timer_interval_set(efl_added, in));
+   td = efl_data_scope_get(timer, MY_CLASS);
+   td->legacy = legacy;
    return timer;
 }
 
@@ -195,6 +203,7 @@ EAPI Ecore_Timer *
 ecore_timer_loop_add(double in, Ecore_Task_Cb func, const void  *data)
 {
    Ecore_Timer_Legacy *legacy;
+   Efl_Loop_Timer_Data *td;
    Eo *timer;
 
    EINA_MAIN_LOOP_CHECK_RETURN_VAL(NULL);
@@ -209,24 +218,26 @@ ecore_timer_loop_add(double in, Ecore_Task_Cb func, const void  *data)
    legacy->data = data;
    timer = efl_add(MY_CLASS, efl_main_loop_get(),
                   efl_event_callback_array_add(efl_added, legacy_timer(), legacy),
-                  efl_key_data_set(efl_added, "_legacy", legacy),
                   efl_loop_timer_loop_reset(efl_added),
                   efl_loop_timer_interval_set(efl_added, in));
+   td = efl_data_scope_get(timer, MY_CLASS);
+   td->legacy = legacy;
    return timer;
 }
 
 EAPI void *
 ecore_timer_del(Ecore_Timer *timer)
 {
-   Ecore_Timer_Legacy *legacy;
+   Efl_Loop_Timer_Data *td;
    void *data;
 
    if (!timer) return NULL;
    EINA_MAIN_LOOP_CHECK_RETURN_VAL(NULL);
 
-   legacy = efl_key_data_get(timer, "_legacy");
+
+   td = efl_data_scope_safe_get(timer, MY_CLASS);
    // If legacy == NULL, this means double free or something
-   if (legacy == NULL)
+   if ((!td) || (!td->legacy))
      {
         // Just in case it is an Eo timer, but not a legacy one.
         ERR("You are trying to destroy a timer which seems dead already.");
@@ -234,8 +245,8 @@ ecore_timer_del(Ecore_Timer *timer)
         return NULL;
      }
 
-   data = (void *)legacy->data;
-   if (legacy->inside_call) legacy->delete_me = EINA_TRUE;
+   data = (void *)td->legacy->data;
+   if (td->legacy->inside_call) td->legacy->delete_me = EINA_TRUE;
    else efl_del(timer);
    return data;
 }
@@ -570,7 +581,8 @@ _efl_loop_timer_next_get(Eo *obj, Efl_Loop_Data *pd)
 static inline void
 _efl_loop_timer_reschedule(Efl_Loop_Timer_Data *timer, double when)
 {
-   if (timer->frozen || efl_invalidated_get(timer->object)) return;
+   if (timer->frozen || efl_invalidated_get(timer->object) ||
+       (timer->legacy && timer->legacy->delete_me)) return;
 
    if (timer->loop_data &&
        (EINA_INLIST_GET(timer)->next || EINA_INLIST_GET(timer)->prev))