Permalink
Browse files

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)
  • Loading branch information...
1 parent 33a80ee commit 75537a93e985ef32e6c267b06ce93629855dd983 @brammool brammool committed Sep 5, 2016
Showing with 130 additions and 81 deletions.
  1. +15 −12 src/evalfunc.c
  2. +79 −66 src/ex_cmds2.c
  3. +1 −1 src/proto/ex_cmds2.pro
  4. +3 −2 src/structs.h
  5. +30 −0 src/testdir/test_timers.vim
  6. +2 −0 src/version.c
View
@@ -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,21 +12420,22 @@ 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)
timer->tr_callback = vim_strsave(callback);
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;
}
}
View
@@ -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);
}
/*
@@ -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);
View
@@ -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 */
@@ -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
View
@@ -764,6 +764,8 @@ static char *(features[]) =
static int included_patches[] =
{ /* Add new patch number below this line */
/**/
+ 2332,
+/**/
2331,
/**/
2330,

0 comments on commit 75537a9

Please sign in to comment.