bug#12447: 24.1.50; Stuck in garbage collection on OS X

[Eli Zaretskii]

> Date: Sun, 16 Sep 2012 19:56:11 +0400
> From: Dmitry Gutov <dgutov@yandex.ru>
> Cc: 12447@debbugs.gnu.org, hanche@math.ntnu.no
> 
> On 16.09.2012 18:54, Eli Zaretskii wrote:
> >> Date: Sun, 16 Sep 2012 18:25:50 +0400
> >> From: Dmitry Gutov <dgutov@yandex.ru>
> >> CC: hanche@math.ntnu.no, 12447@debbugs.gnu.org
> >>
> >>> Perhaps the problem is that the value of 'difference' is not
> >>> initialized:
> >>>
> >>>     while (CONSP (timers) || CONSP (idle_timers))
> >>>       {
> >>>         Lisp_Object timer = Qnil, idle_timer = Qnil;
> >>>         EMACS_TIME timer_time, idle_timer_time;
> >>>         EMACS_TIME difference;   <<<<<<<<<<<<<<<<<<<<<<<<<<<<
> >>>
> >>> and then never set to any specific value, until here:
> >>>
> >>>         else
> >>>   /* When we encounter a timer that is still waiting,
> >>>      return the amount of time to wait before it is ripe.  */
> >>>   {
> >>>     UNGCPRO;
> >>>     return difference;
> >>>   }
> >>>
> >>> which causes us return garbage, potentially zero, to timer_check.
> >>
> >> It's assigned to, though. When we encounter a timer that's not yet ripe.
> >
> > What if all of them are ripe?
> 
> I don't see the problem. The first timer is ripe? Fire it and return 
> 'nexttime'. Otherwise, return 'difference', which now has been assigned 
> a value.
> If we've reached the end of the list, again return 'nexttime', which is 
> initialized with invalid_emacs_time () at the beginning of timer_check_2.
> 
> Anyway, I think the immediate problem is that the newly created timer 
> can be considered ripe.

The patch below makes your simplified recipe, viz.:

  (defvar counter 0)

  (defun foo ()
    (message (format  "foo %s" counter))
    (setq counter (1+ counter))
    (run-with-idle-timer 1 nil #'foo))
  (foo)

"work" without locking up Emacs.  "Work" in the sense that the timer
is run and increments the counter, but keyboard input is still
accepted, and causes 1-sec break in the idle timer invocation.  What
does NOT happen is the once-per-second invocation of the idle timer:
as long as there's no other input, the idle timer runs much more
frequently.  But I think this is expected, since the call to
run-with-idle-timer above explicitly asks to be run immediately.

Can you see if these changes also make js2-mode work as expected?

Jan, can you test whether this patch still keeps your two-timers
recipe working?  If it does, I think I should commit the changes
below, because they avoid locking up Emacs by a timer that repeatedly
reinvokes itself.


=== modified file 'src/keyboard.c'
--- src/keyboard.c      2012-09-16 21:43:55 +0000
+++ src/keyboard.c      2012-09-18 14:58:24 +0000
@@ -4334,25 +4334,18 @@ decode_timer (Lisp_Object timer, EMACS_T
    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))
@@ -4501,13 +4494,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;
 }