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)

src/evalfunc.c

@@ -12403,7 +12403,9 @@ f_timer_start(typval_T *argvars, typval_T *rettv)
     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], &partial);
-    callback = get_callback(&argvars[1], &timer->tr_partial);
     if (callback == NULL)
-    {
+	return;
-	stop_timer(timer);
+
-	rettv->vval.v_number = -1;
+    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;
     }
 }
 

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;
+# ifdef WIN3264
-static int	free_current_timer = FALSE;
+#  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.
@@ -1123,15 +1130,10 @@ 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);
 }
-}
 
 /*
  * Create a timer and return it.  NULL if out of memory.
@@ -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,78 +1187,77 @@ 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);
-	next_due = -1;
+    for (timer = first_timer; timer != NULL && !got_int; timer = timer_next)
-	for (timer = first_timer; timer != NULL; timer = timer->tr_next)
     {
-	    if (timer->tr_paused)
+	timer_next = timer->tr_next;
+
+	if (timer->tr_id == -1 || timer->tr_firing || timer->tr_paused)
 	    continue;
-# ifdef WIN3264
+	this_due = GET_TIMEDIFF(timer, now);
-	    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)
 	{
-		current_timer = timer;
+	    timer->tr_firing = TRUE;
-		free_current_timer = FALSE;
 	    timer_callback(timer);
-		current_timer = NULL;
+	    timer->tr_firing = FALSE;
-
+	    timer_next = timer->tr_next;
 	    did_one = TRUE;
-		if (timer->tr_repeat != 0 && !free_current_timer)
+
+	    /* 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)
 	    {
 		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);
 	    }
-		/* the callback may do anything, start all over */
-		break;
 	}
-	    if (next_due == -1 || next_due > this_due)
+	if (this_due > 0 && (next_due == -1 || next_due > this_due))
 	    next_due = this_due;
     }
-	if (timer == NULL)
-	    break;
-    }
 
     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;
 
+    if (id >= 0)
+    {
 	for (timer = first_timer; timer != NULL; timer = timer->tr_next)
 	    if (timer->tr_id == id)
-	    break;
 		return timer;
     }
+    return NULL;
+}
 
 
 /*
@@ -1261,16 +1265,28 @@ find_timer(int id)
  */
     void
 stop_timer(timer_T *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)
+    timer_T *timer;
-	stop_timer(first_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,6 +1345,7 @@ add_timer_info_all(typval_T *rettv)
     timer_T *timer;
 
     for (timer = first_timer; timer != NULL; timer = timer->tr_next)
+	if (timer->tr_id != -1)
 	    add_timer_info(rettv, timer);
 }
 

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);

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 */

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

src/version.c

@@ -763,6 +763,8 @@ static char *(features[]) =
 
 static int included_patches[] =
 {   /* Add new patch number below this line */
+/**/
+    2332,
 /**/
     2331,
 /**/