From df9685f3961022245b9ab73b62023aa573862001 Mon Sep 17 00:00:00 2001 From: Eli Zaretskii Date: Sat, 22 Sep 2012 16:16:03 +0300 Subject: [PATCH] Fix bugs #12447 and #12326 with infloop causes by idle timers, update docs. src/keyboard.c (timer_check_2): Move calculation of 'timers' and 'idle_timers' from here ... (timer_check): ... to here. Use Fcopy_sequence to copy the timer lists, to avoid infloops when the timer does something stupid, like reinvoke itself with the same or smaller time-out. lisp/emacs-lisp/timer.el (run-with-idle-timer) (timer-activate-when-idle): Warn against reinvoking an idle timer from within its own timer action. doc/lispref/os.texi (Idle Timers): Warn against reinvoking an idle timer from within its own timer action. --- doc/lispref/ChangeLog | 5 +++++ doc/lispref/os.texi | 7 +++++++ lisp/ChangeLog | 6 ++++++ lisp/emacs-lisp/timer.el | 15 ++++++++++++--- src/ChangeLog | 9 +++++++++ src/keyboard.c | 34 ++++++++++++++++++++++------------ 6 files changed, 61 insertions(+), 15 deletions(-) diff --git a/doc/lispref/ChangeLog b/doc/lispref/ChangeLog index f1ae632267b..8acd12d82a8 100644 --- a/doc/lispref/ChangeLog +++ b/doc/lispref/ChangeLog @@ -1,3 +1,8 @@ +2012-09-22 Eli Zaretskii + + * os.texi (Idle Timers): Warn against reinvoking an idle timer + from within its own timer action. (Bug#12447) + 2012-09-22 Chong Yidong * frames.texi (Pop-Up Menus): Minor clarification (Bug#11148). diff --git a/doc/lispref/os.texi b/doc/lispref/os.texi index 6431ac8bead..68e53c78972 100644 --- a/doc/lispref/os.texi +++ b/doc/lispref/os.texi @@ -1863,6 +1863,13 @@ only while waiting). It blocks out any idle timers that ought to run during that time. @end itemize +@noindent +For similar reasons, do not write an idle timer function that sets +up another idle time (including the same idle timer) with the +@var{secs} argument less or equal to the current idleness time. Such +a timer will run almost immediately, and continue running again and +again, instead of waiting for the next time Emacs becomes idle. + @noindent The correct approach is for the idle timer to reschedule itself after a brief pause, using the method in the @code{timer-function} example diff --git a/lisp/ChangeLog b/lisp/ChangeLog index f18bfd73611..b0e91b675a3 100644 --- a/lisp/ChangeLog +++ b/lisp/ChangeLog @@ -1,3 +1,9 @@ +2012-09-22 Eli Zaretskii + + * emacs-lisp/timer.el (run-with-idle-timer) + (timer-activate-when-idle): Warn against reinvoking an idle timer + from within its own timer action. (Bug#12447) + 2012-09-22 Martin Rudalics * cus-start.el (window-combination-limit): Add new optional diff --git a/lisp/emacs-lisp/timer.el b/lisp/emacs-lisp/timer.el index 2248dde8c03..bcd582a6f88 100644 --- a/lisp/emacs-lisp/timer.el +++ b/lisp/emacs-lisp/timer.el @@ -205,12 +205,19 @@ timers). If nil, allocate a new cell." "Insert TIMER into `timer-idle-list'. This arranges to activate TIMER whenever Emacs is next idle. If optional argument DONT-WAIT is non-nil, set TIMER to activate -immediately, or at the right time, if Emacs is already idle. +immediately \(see beloe\), or at the right time, if Emacs is +already idle. REUSE-CELL, if non-nil, is a cons cell to reuse when inserting TIMER into `timer-idle-list' (usually a cell removed from that list by `cancel-timer-internal'; using this reduces consing for -repeat timers). If nil, allocate a new cell." +repeat timers). If nil, allocate a new cell. + +Using non-nil DONT-WAIT is not recommended when activating an +idle timer from an idle timer handler, if the timer being +activated has an idleness time that is smaller or equal to +the time of the current timer. That's because the activated +timer will fire right away." (timer--activate timer (not dont-wait) reuse-cell 'idle)) (defalias 'disable-timeout 'cancel-timer) @@ -403,7 +410,9 @@ The action is to call FUNCTION with arguments ARGS. SECS may be an integer, a floating point number, or the internal time format returned by, e.g., `current-idle-time'. If Emacs is currently idle, and has been idle for N seconds (N < SECS), -then it will call FUNCTION in SECS - N seconds from now. +then it will call FUNCTION in SECS - N seconds from now. Using +SECS <= N is not recommended if this function is invoked from an idle +timer, because FUNCTION will then be called immediately. If REPEAT is non-nil, do the action each time Emacs has been idle for exactly SECS seconds (that is, only once for each time Emacs becomes idle). diff --git a/src/ChangeLog b/src/ChangeLog index 6ea40b3f122..b69d4bb7113 100644 --- a/src/ChangeLog +++ b/src/ChangeLog @@ -1,3 +1,12 @@ +2012-09-22 Eli Zaretskii + + * keyboard.c (timer_check_2): Move calculation of 'timers' and + 'idle_timers' from here ... + (timer_check): ... to here. Use Fcopy_sequence to copy the timer + lists, to avoid infloops when the timer does something stupid, + like reinvoke itself with the same or smaller time-out. + (Bug#12447) + 2012-09-22 Martin Rudalics * window.c (Fsplit_window_internal): Handle only Qt value of diff --git a/src/keyboard.c b/src/keyboard.c index 098d3530ef8..8b1113a026a 100644 --- a/src/keyboard.c +++ b/src/keyboard.c @@ -4333,25 +4333,18 @@ decode_timer (Lisp_Object timer, EMACS_TIME *result) should be done. */ static EMACS_TIME -timer_check_2 (void) +timer_check_2 (Lisp_Object timers, Lisp_Object idle_timers) { EMACS_TIME nexttime; EMACS_TIME now; EMACS_TIME idleness_now; - Lisp_Object timers, idle_timers, chosen_timer; - struct gcpro gcpro1, gcpro2, gcpro3; + Lisp_Object chosen_timer; + struct gcpro gcpro1; nexttime = invalid_emacs_time (); - /* Always consider the ordinary timers. */ - timers = Vtimer_list; - /* Consider the idle timers only if Emacs is idle. */ - if (EMACS_TIME_VALID_P (timer_idleness_start_time)) - idle_timers = Vtimer_idle_list; - else - idle_timers = Qnil; chosen_timer = Qnil; - GCPRO3 (timers, idle_timers, chosen_timer); + GCPRO1 (chosen_timer); /* First run the code that was delayed. */ while (CONSP (pending_funcalls)) @@ -4500,13 +4493,30 @@ EMACS_TIME timer_check (void) { EMACS_TIME nexttime; + Lisp_Object timers, idle_timers; + struct gcpro gcpro1, gcpro2; + + /* We use copies of the timers' lists to allow a timer to add itself + again, without locking up Emacs if the newly added timer is + already ripe when added. */ + + /* Always consider the ordinary timers. */ + timers = Fcopy_sequence (Vtimer_list); + /* Consider the idle timers only if Emacs is idle. */ + if (EMACS_TIME_VALID_P (timer_idleness_start_time)) + idle_timers = Fcopy_sequence (Vtimer_idle_list); + else + idle_timers = Qnil; + + GCPRO2 (timers, idle_timers); do { - nexttime = timer_check_2 (); + nexttime = timer_check_2 (timers, idle_timers); } while (EMACS_SECS (nexttime) == 0 && EMACS_NSECS (nexttime) == 0); + UNGCPRO; return nexttime; } -- 2.39.2