# HG changeset patch # User Christian Brabandt # Date 1473109207 -7200 # Node ID 3db463d4df255c365fb4241027932b7ff7412f7b # Parent 02ba0b8a13fbf7db0e5107770fefb1575f870791 commit https://github.com/vim/vim/commit/75537a93e985ef32e6c267b06ce93629855dd983 Author: Bram Moolenaar Date: Mon Sep 5 22:45:28 2016 +0200 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 --- a/src/evalfunc.c +++ b/src/evalfunc.c @@ -12398,12 +12398,14 @@ f_timer_pause(typval_T *argvars, typval_ 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_ 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_ 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 --- 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 timer_T *current_timer = NULL; -static int free_current_timer = FALSE; +static long last_timer_id = 0; + +# 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; } - if (next_due == -1 || next_due > this_due) - next_due = this_due; + else + { + this_due = -1; + remove_timer(timer); + free_timer(timer); + } } - 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 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 --- 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 --- 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 --- 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 --- a/src/version.c +++ b/src/version.c @@ -764,6 +764,8 @@ static char *(features[]) = static int included_patches[] = { /* Add new patch number below this line */ /**/ + 2332, +/**/ 2331, /**/ 2330,