patch 7.4.2332

Problem:    Crash when stop_timer() is called in a callback of a callback.
            Vim hangs when the timer callback uses too much time.
Solution:   Set tr_id to -1 when a timer is to be deleted. Don't keep calling
            callbacks forever. (Ozaki Kiichi)

diff --git a/src/evalfunc.c b/src/evalfunc.c
index 1c1dcf71202428d7dd344e0fb1a54211613abbe6..906fa39afc90546474b5438c519ec4f1093223e9 100644 (file)
--- a/src/evalfunc.c
+++ b/src/evalfunc.c
@@ -12398,12 +12398,14 @@ f_timer_pause(typval_T *argvars, typval_T *rettv UNUSED)
     static void
 f_timer_start(typval_T *argvars, typval_T *rettv)
 {
-    long    msec = (long)get_tv_number(&argvars[0]);
-    timer_T *timer;
-    int            repeat = 0;
-    char_u  *callback;
-    dict_T  *dict;
+    long       msec = (long)get_tv_number(&argvars[0]);
+    timer_T    *timer;
+    int                repeat = 0;
+    char_u     *callback;
+    dict_T     *dict;
+    partial_T  *partial;
 
+    rettv->vval.v_number = -1;
     if (check_secure())
        return;
     if (argvars[2].v_type != VAR_UNKNOWN)
@@ -12418,13 +12420,13 @@ f_timer_start(typval_T *argvars, typval_T *rettv)
            repeat = get_dict_number(dict, (char_u *)"repeat");
     }
 
-    timer = create_timer(msec, repeat);
-    callback = get_callback(&argvars[1], &timer->tr_partial);
+    callback = get_callback(&argvars[1], &partial);
     if (callback == NULL)
-    {
-       stop_timer(timer);
-       rettv->vval.v_number = -1;
-    }
+       return;
+
+    timer = create_timer(msec, repeat);
+    if (timer == NULL)
+       free_callback(callback, partial);
     else
     {
        if (timer->tr_partial == NULL)
@@ -12432,7 +12434,8 @@ f_timer_start(typval_T *argvars, typval_T *rettv)
        else
            /* pointer into the partial */
            timer->tr_callback = callback;
-       rettv->vval.v_number = timer->tr_id;
+       timer->tr_partial = partial;
+       rettv->vval.v_number = (varnumber_T)timer->tr_id;
     }
 }
 

diff --git a/src/ex_cmds2.c b/src/ex_cmds2.c
index acaef9fcef4b524963d2d8baa5d004a82d8acd94..8df67536c55c0b74bd386ef0a3072d1c402a9c50 100644 (file)
--- a/src/ex_cmds2.c
+++ b/src/ex_cmds2.c
@@ -1088,10 +1088,17 @@ profile_zero(proftime_T *tm)
 
 # if defined(FEAT_TIMERS) || defined(PROTO)
 static timer_T *first_timer = NULL;
-static int     last_timer_id = 0;
+static long    last_timer_id = 0;
 
-static timer_T *current_timer = NULL;
-static int     free_current_timer = FALSE;
+# ifdef WIN3264
+#  define GET_TIMEDIFF(timer, now) \
+       (long)(((double)(timer->tr_due.QuadPart - now.QuadPart) \
+                                          / (double)fr.QuadPart) * 1000);
+# else
+#  define GET_TIMEDIFF(timer, now) \
+       (timer->tr_due.tv_sec - now.tv_sec) * 1000 \
+                          + (timer->tr_due.tv_usec - now.tv_usec) / 1000;
+# endif
 
 /*
  * Insert a timer in the list of timers.
@@ -1124,13 +1131,8 @@ remove_timer(timer_T *timer)
     static void
 free_timer(timer_T *timer)
 {
-    if (timer == current_timer)
-       free_current_timer = TRUE;
-    else
-    {
-       free_callback(timer->tr_callback, timer->tr_partial);
-       vim_free(timer);
-    }
+    free_callback(timer->tr_callback, timer->tr_partial);
+    vim_free(timer);
 }
 
 /*
@@ -1144,7 +1146,10 @@ create_timer(long msec, int repeat)
 
     if (timer == NULL)
        return NULL;
-    timer->tr_id = ++last_timer_id;
+    if (++last_timer_id < 0)
+       /* Overflow!  Might cause duplicates... */
+       last_timer_id = 0;
+    timer->tr_id = last_timer_id;
     insert_timer(timer);
     if (repeat != 0)
        timer->tr_repeat = repeat - 1;
@@ -1165,7 +1170,7 @@ timer_callback(timer_T *timer)
     typval_T   argv[2];
 
     argv[0].v_type = VAR_NUMBER;
-    argv[0].vval.v_number = timer->tr_id;
+    argv[0].vval.v_number = (varnumber_T)timer->tr_id;
     argv[1].v_type = VAR_UNKNOWN;
 
     call_func(timer->tr_callback, (int)STRLEN(timer->tr_callback),
@@ -1182,77 +1187,76 @@ timer_callback(timer_T *timer)
 check_due_timer(void)
 {
     timer_T    *timer;
+    timer_T    *timer_next;
     long       this_due;
     long       next_due = -1;
     proftime_T now;
     int                did_one = FALSE;
+    long       current_id = last_timer_id;
 # ifdef WIN3264
     LARGE_INTEGER   fr;
 
     QueryPerformanceFrequency(&fr);
 # endif
-    while (!got_int)
+    profile_start(&now);
+    for (timer = first_timer; timer != NULL && !got_int; timer = timer_next)
     {
-       profile_start(&now);
-       next_due = -1;
-       for (timer = first_timer; timer != NULL; timer = timer->tr_next)
+       timer_next = timer->tr_next;
+
+       if (timer->tr_id == -1 || timer->tr_firing || timer->tr_paused)
+           continue;
+       this_due = GET_TIMEDIFF(timer, now);
+       if (this_due <= 1)
        {
-           if (timer->tr_paused)
-               continue;
-# ifdef WIN3264
-           this_due = (long)(((double)(timer->tr_due.QuadPart - now.QuadPart)
-                                              / (double)fr.QuadPart) * 1000);
-# else
-           this_due = (timer->tr_due.tv_sec - now.tv_sec) * 1000
-                              + (timer->tr_due.tv_usec - now.tv_usec) / 1000;
-# endif
-           if (this_due <= 1)
+           timer->tr_firing = TRUE;
+           timer_callback(timer);
+           timer->tr_firing = FALSE;
+           timer_next = timer->tr_next;
+           did_one = TRUE;
+
+           /* Only fire the timer again if it repeats and stop_timer() wasn't
+            * called while inside the callback (tr_id == -1). */
+           if (timer->tr_repeat != 0 && timer->tr_id != -1)
            {
-               current_timer = timer;
-               free_current_timer = FALSE;
-               timer_callback(timer);
-               current_timer = NULL;
-
-               did_one = TRUE;
-               if (timer->tr_repeat != 0 && !free_current_timer)
-               {
-                   profile_setlimit(timer->tr_interval, &timer->tr_due);
-                   if (timer->tr_repeat > 0)
-                       --timer->tr_repeat;
-               }
-               else
-               {
-                   remove_timer(timer);
-                   free_timer(timer);
-               }
-               /* the callback may do anything, start all over */
-               break;
+               profile_setlimit(timer->tr_interval, &timer->tr_due);
+               this_due = GET_TIMEDIFF(timer, now);
+               if (this_due < 1)
+                   this_due = 1;
+               if (timer->tr_repeat > 0)
+                   --timer->tr_repeat;
+           }
+           else
+           {
+               this_due = -1;
+               remove_timer(timer);
+               free_timer(timer);
            }
-           if (next_due == -1 || next_due > this_due)
-               next_due = this_due;
        }
-       if (timer == NULL)
-           break;
+       if (this_due > 0 && (next_due == -1 || next_due > this_due))
+           next_due = this_due;
     }
 
     if (did_one)
        redraw_after_callback();
 
-    return next_due;
+    return current_id != last_timer_id ? 1 : next_due;
 }
 
 /*
  * Find a timer by ID.  Returns NULL if not found;
  */
     timer_T *
-find_timer(int id)
+find_timer(long id)
 {
     timer_T *timer;
 
-    for (timer = first_timer; timer != NULL; timer = timer->tr_next)
-       if (timer->tr_id == id)
-           break;
-    return timer;
+    if (id >= 0)
+    {
+       for (timer = first_timer; timer != NULL; timer = timer->tr_next)
+           if (timer->tr_id == id)
+               return timer;
+    }
+    return NULL;
 }
 
 
@@ -1262,15 +1266,27 @@ find_timer(int id)
     void
 stop_timer(timer_T *timer)
 {
-    remove_timer(timer);
-    free_timer(timer);
+    if (timer->tr_firing)
+       /* Free the timer after the callback returns. */
+       timer->tr_id = -1;
+    else
+    {
+       remove_timer(timer);
+       free_timer(timer);
+    }
 }
 
     void
 stop_all_timers(void)
 {
-    while (first_timer != NULL)
-       stop_timer(first_timer);
+    timer_T *timer;
+    timer_T *timer_next;
+
+    for (timer = first_timer; timer != NULL; timer = timer_next)
+    {
+       timer_next = timer->tr_next;
+       stop_timer(timer);
+    }
 }
 
     void
@@ -1289,18 +1305,14 @@ add_timer_info(typval_T *rettv, timer_T *timer)
        return;
     list_append_dict(list, dict);
 
-    dict_add_nr_str(dict, "id", (long)timer->tr_id, NULL);
+    dict_add_nr_str(dict, "id", timer->tr_id, NULL);
     dict_add_nr_str(dict, "time", (long)timer->tr_interval, NULL);
 
     profile_start(&now);
 # ifdef WIN3264
     QueryPerformanceFrequency(&fr);
-    remaining = (long)(((double)(timer->tr_due.QuadPart - now.QuadPart)
-                                              / (double)fr.QuadPart) * 1000);
-# else
-    remaining = (timer->tr_due.tv_sec - now.tv_sec) * 1000
-                              + (timer->tr_due.tv_usec - now.tv_usec) / 1000;
 # endif
+    remaining = GET_TIMEDIFF(timer, now);
     dict_add_nr_str(dict, "remaining", (long)remaining, NULL);
 
     dict_add_nr_str(dict, "repeat",
@@ -1333,7 +1345,8 @@ add_timer_info_all(typval_T *rettv)
     timer_T *timer;
 
     for (timer = first_timer; timer != NULL; timer = timer->tr_next)
-       add_timer_info(rettv, timer);
+       if (timer->tr_id != -1)
+           add_timer_info(rettv, timer);
 }
 
 /*

diff --git a/src/proto/ex_cmds2.pro b/src/proto/ex_cmds2.pro
index f241d377e8d4f4867e4749c4c0128ac1e144be96..c1325a41e3bb432d2ce53980832d6c28d8867d35 100644 (file)
--- a/src/proto/ex_cmds2.pro
+++ b/src/proto/ex_cmds2.pro
@@ -20,7 +20,7 @@ int profile_passed_limit(proftime_T *tm);
 void profile_zero(proftime_T *tm);
 timer_T *create_timer(long msec, int repeat);
 long check_due_timer(void);
-timer_T *find_timer(int id);
+timer_T *find_timer(long id);
 void stop_timer(timer_T *timer);
 void stop_all_timers(void);
 void add_timer_info(typval_T *rettv, timer_T *timer);

diff --git a/src/structs.h b/src/structs.h
index a44bcec4cbc79b27039ab4c279706413b3f96ac6..8bdd3c9cb3bee1dad6a552bf29940e892a1f8354 100644 (file)
--- a/src/structs.h
+++ b/src/structs.h
@@ -3166,12 +3166,13 @@ typedef struct js_reader js_read_T;
 typedef struct timer_S timer_T;
 struct timer_S
 {
-    int                tr_id;
+    long       tr_id;
 #ifdef FEAT_TIMERS
     timer_T    *tr_next;
     timer_T    *tr_prev;
     proftime_T tr_due;             /* when the callback is to be invoked */
-    int                tr_paused;          /* when TRUE callback is not invoked */
+    char       tr_firing;          /* when TRUE callback is being called */
+    char       tr_paused;          /* when TRUE callback is not invoked */
     int                tr_repeat;          /* number of times to repeat, -1 forever */
     long       tr_interval;        /* msec */
     char_u     *tr_callback;       /* allocated */

diff --git a/src/testdir/test_timers.vim b/src/testdir/test_timers.vim
index e6e54a4cbdc4ee646149588fa35577fa9d8f0daf..07c6876eeeae8ba02713e035fbf5d3f840dcaf31 100644 (file)
--- a/src/testdir/test_timers.vim
+++ b/src/testdir/test_timers.vim
@@ -143,4 +143,34 @@ func Test_delete_myself()
   call assert_equal([], timer_info(t))
 endfunc
 
+func StopTimer1(timer)
+  let g:timer2 = timer_start(10, 'StopTimer2')
+  " avoid maxfuncdepth error
+  call timer_pause(g:timer1, 1)
+  sleep 40m
+endfunc
+
+func StopTimer2(timer)
+  call timer_stop(g:timer1)
+endfunc
+
+func Test_stop_in_callback()
+  let g:timer1 = timer_start(10, 'StopTimer1')
+  sleep 40m
+endfunc
+
+func StopTimerAll(timer)
+  call timer_stopall()
+endfunc
+
+func Test_stop_all_in_callback()
+  let g:timer1 = timer_start(10, 'StopTimerAll')
+  let info = timer_info()
+  call assert_equal(1, len(info))
+  sleep 40m
+  let info = timer_info()
+  call assert_equal(0, len(info))
+endfunc
+
+
 " vim: shiftwidth=2 sts=2 expandtab

diff --git a/src/version.c b/src/version.c
index 740d978e90861301d163496aded9a924a58e03b6..98b722562806d0b295823905626ab1d69760cf31 100644 (file)
--- a/src/version.c
+++ b/src/version.c
@@ -763,6 +763,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    2332,
 /**/
     2331,
 /**/