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.

diff --git a/doc/lispref/ChangeLog b/doc/lispref/ChangeLog
index f1ae632267b7c3cd8d6674557c5c87b18ea1138c..8acd12d82a8bc702c172957d0688c1d88cdaaa13 100644 (file)
--- a/doc/lispref/ChangeLog
+++ b/doc/lispref/ChangeLog
@@ -1,3 +1,8 @@
+2012-09-22  Eli Zaretskii  <eliz@gnu.org>
+
+       * os.texi (Idle Timers): Warn against reinvoking an idle timer
+       from within its own timer action.  (Bug#12447)
+
 2012-09-22  Chong Yidong  <cyd@gnu.org>
 
        * frames.texi (Pop-Up Menus): Minor clarification (Bug#11148).

diff --git a/doc/lispref/os.texi b/doc/lispref/os.texi
index 6431ac8beadf06a4f7cf1aa9bdfa9d90da6ba075..68e53c78972c1f57a53195f69c6e9a90a6efb40b 100644 (file)
--- 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 f18bfd7361104d2768cf939b3c3b5b1b6aa60ebe..b0e91b675a3a137e21392fc1be415bfca550effc 100644 (file)
--- a/lisp/ChangeLog
+++ b/lisp/ChangeLog
@@ -1,3 +1,9 @@
+2012-09-22  Eli Zaretskii  <eliz@gnu.org>
+
+       * 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  <rudalics@gmx.at>
 
        * cus-start.el (window-combination-limit): Add new optional

diff --git a/lisp/emacs-lisp/timer.el b/lisp/emacs-lisp/timer.el
index 2248dde8c03b2624f45dbd634b69da1eff03d801..bcd582a6f8875530242cd90f99d4b024431e47c7 100644 (file)
--- 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 6ea40b3f122a47c0360e116106047a4e1aff135f..b69d4bb711346ad73e64265b76d60b03da522e7c 100644 (file)
--- a/src/ChangeLog
+++ b/src/ChangeLog
@@ -1,3 +1,12 @@
+2012-09-22  Eli Zaretskii  <eliz@gnu.org>
+
+       * 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  <rudalics@gmx.at>
 
        * window.c (Fsplit_window_internal): Handle only Qt value of

diff --git a/src/keyboard.c b/src/keyboard.c
index 098d3530ef845d267ed5c09d19d8c3e6d6f70ec3..8b1113a026af7f444a3547545f0088e43fc6d4dd 100644 (file)
--- 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;
 }