Re: Nested sit-for's

[Chong Yidong]

address@hidden (Kim F. Storm) writes:

> (defun st1 ()
>   (with-current-buffer (get-buffer-create "*st1*")
>     (goto-char (point-max))
>     (insert "<")
>     (sit-for 30)
>     (insert ">")))
>
> (run-with-timer 1 2 'st1)
>
> (progn
>   (message "sit-for...")
>   (sit-for 5)
>   (message "sit-for...done"))
>
> Now, the sit-for...done message is shown after 30-35 seconds,
> not after 5 seconds...
>
> The call to sit-for in the timer is probably "bad practice", but it
> could just as well have happened in a process filter or some other
> async handler.

How about the following patch?  The preemption code takes place
read_filtered_event, so it affects nested `sit-for's while leaving
unchanged other uses of wait_reading_process_output, such as the
sit_for() function called in a handful of places in the C code.  I am
unsure whether it is important/correct/desirable to preempt those
cases.  Opinions welcome.

There's one part of the patch that I think may be incorrect.  It saves
the seconds and microseconds components of the end time (originally an
EMACS_TIME) in two Lisp integers, which is probably not big enough.  I
think the solution is to introduce a few utility functions to convert
between EMACS_TIMEs and the Lisp-level (HIGH LOW USECS) time
representation.  (This may let us reimplement `timer-relative-time'
more straightforwardly too).

*** emacs/src/lread.c.~1.362.~  2006-07-26 13:45:54.000000000 -0400
--- emacs/src/lread.c   2006-08-16 14:46:50.000000000 -0400
***************
*** 439,444 ****
--- 439,457 ----
  
  extern Lisp_Object read_char ();
  
+ static EMACS_TIME saved_end_time;
+ 
+ static Lisp_Object
+ read_filtered_event_unwind (data)
+      Lisp_Object data;
+ {
+   if (!NILP (data))
+     EMACS_SET_SECS_USECS (saved_end_time,
+                         XINT (XCAR (data)),
+                         XINT (XCDR (data)));
+   return Qnil;
+ }
+ 
  /* Read input events until we get one that's acceptable for our purposes.
  
     If NO_SWITCH_FRAME is non-zero, switch-frame events are stashed
***************
*** 468,473 ****
--- 481,487 ----
  {
    Lisp_Object val, delayed_switch_frame;
    EMACS_TIME end_time;
+   int count = SPECPDL_INDEX ();
  
  #ifdef HAVE_WINDOW_SYSTEM
    if (display_hourglass_p)
***************
*** 488,493 ****
--- 502,522 ----
        EMACS_GET_TIME (end_time);
        EMACS_SET_SECS_USECS (wait_time, sec, usec);
        EMACS_ADD_TIME (end_time, end_time, wait_time);
+ 
+       if (!EMACS_TIME_NEG_P (saved_end_time)
+         && EMACS_TIME_GE (end_time, saved_end_time))
+       {
+         EMACS_SET_SECS  (end_time, EMACS_SECS  (saved_end_time));
+         EMACS_SET_USECS (end_time, EMACS_USECS (saved_end_time));
+       }
+       else
+       {
+         record_unwind_protect (read_filtered_event_unwind, 
+                                Fcons (make_number (EMACS_SECS 
(saved_end_time)),
+                                       make_number (EMACS_USECS 
(saved_end_time))));
+         EMACS_SET_SECS  (saved_end_time, EMACS_SECS  (end_time));
+         EMACS_SET_USECS (saved_end_time, EMACS_USECS (end_time));
+       }
      }
  
    /* Read until we get an acceptable event.  */
***************
*** 553,558 ****
--- 582,588 ----
  
  #endif
  
+   unbind_to (count, Qnil);
    return val;
  }
  
***************
*** 4230,4235 ****
--- 4260,4267 ----
  
    Vloads_in_progress = Qnil;
    staticpro (&Vloads_in_progress);
+ 
+   EMACS_SET_SECS_USECS (saved_end_time, -1, -1);
  }
  
  /* arch-tag: a0d02733-0f96-4844-a659-9fd53c4f414d