[Emacs-diffs] /srv/bzr/emacs/trunk r110138: Fix bugs #12447 and #12326 with infloop causes by idle timers, update docs.

[Eli Zaretskii]

------------------------------------------------------------
revno: 110138
fixes bugs: http://debbugs.gnu.org/12447 http://debbugs.gnu.org/12326
committer: Eli Zaretskii <address@hidden>
branch nick: trunk
timestamp: Sat 2012-09-22 16:16:03 +0300
message:
  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.
modified:
  doc/lispref/ChangeLog
  doc/lispref/os.texi
  lisp/ChangeLog
  lisp/emacs-lisp/timer.el
  src/ChangeLog
  src/keyboard.c

=== modified file 'doc/lispref/ChangeLog'
--- a/doc/lispref/ChangeLog     2012-09-22 03:29:37 +0000
+++ b/doc/lispref/ChangeLog     2012-09-22 13:16:03 +0000
@@ -1,3 +1,8 @@
+2012-09-22  Eli Zaretskii  <address@hidden>
+
+       * os.texi (Idle Timers): Warn against reinvoking an idle timer
+       from within its own timer action.  (Bug#12447)
+
 2012-09-22  Chong Yidong  <address@hidden>
 
        * frames.texi (Pop-Up Menus): Minor clarification (Bug#11148).

=== modified file 'doc/lispref/os.texi'
--- a/doc/lispref/os.texi       2012-06-22 21:17:42 +0000
+++ b/doc/lispref/os.texi       2012-09-22 13:16:03 +0000
@@ -1864,6 +1864,13 @@
 @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
address@hidden 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.
+
address@hidden
 The correct approach is for the idle timer to reschedule itself after
 a brief pause, using the method in the @code{timer-function} example
 above.

=== modified file 'lisp/ChangeLog'
--- a/lisp/ChangeLog    2012-09-22 12:56:08 +0000
+++ b/lisp/ChangeLog    2012-09-22 13:16:03 +0000
@@ -1,3 +1,9 @@
+2012-09-22  Eli Zaretskii  <address@hidden>
+
+       * 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  <address@hidden>
 
        * cus-start.el (window-combination-limit): Add new optional

=== modified file 'lisp/emacs-lisp/timer.el'
--- a/lisp/emacs-lisp/timer.el  2012-09-13 16:23:06 +0000
+++ b/lisp/emacs-lisp/timer.el  2012-09-22 13:16:03 +0000
@@ -205,12 +205,19 @@
   "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 @@
 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).

=== modified file 'src/ChangeLog'
--- a/src/ChangeLog     2012-09-22 12:56:08 +0000
+++ b/src/ChangeLog     2012-09-22 13:16:03 +0000
@@ -1,3 +1,12 @@
+2012-09-22  Eli Zaretskii  <address@hidden>
+
+       * 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  <address@hidden>
 
        * window.c (Fsplit_window_internal): Handle only Qt value of

=== modified file 'src/keyboard.c'
--- a/src/keyboard.c    2012-09-20 01:37:07 +0000
+++ b/src/keyboard.c    2012-09-22 13:16:03 +0000
@@ -4333,25 +4333,18 @@
    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 @@
 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;
 }