bug#61741: 30.0.50; [PATCH] Reset errant timers

[Po Lu]

dick.r.chiang@gmail.com writes:

> +(defsubst timer-activate (timer &optional _triggered-p _reuse-cell)
> +  "Install TIMER."
> +  (timer--check timer)
> +  (cl-pushnew timer timer-list))
> +
> +(defsubst timer-activate-when-idle (timer &optional _dont-wait _reuse-cell)
> +  "Install idle TIMER"
> +  (setf (timer--idle-delay timer) 'idle)
> +  (timer--check timer)
> +  (cl-pushnew timer timer-idle-list))
>
>  (defalias 'disable-timeout #'cancel-timer)
>
>  (defun cancel-timer (timer)
>    "Remove TIMER from the list of active timers."
> -  (timer--check timer)
>    (setq timer-list (delq timer timer-list))
>    (setq timer-idle-list (delq timer timer-idle-list))
>    nil)
>

The more doc strings and comments you delete, the less likely it is for
anyone to install your change.  Quit wasting others' time!

>  (defun run-with-idle-timer (secs repeat function &rest args)
> -  "Perform an action the next time Emacs is idle for SECS seconds.
> -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.  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).
> -
> -This function returns a timer object which you can use in `cancel-timer'."
> +  "Call FUNCTION on ARGS when idle for SECS seconds.
> +If REPEAT is non-nil, repeat the behavior until cancelled via
> +`cancel-timer'.  SECS may be an integer, a floating point number,
> +or the internal time format returned by, e.g.,
> +`current-idle-time'."

Here too.  You also deleted the part of the doc string that explains
what to give to cancel-timer.

> -
> +
>  (defvar with-timeout-timers nil
>    "List of all timers used by currently pending `with-timeout' calls.")
>
> @@ -533,7 +431,7 @@ y-or-n-p-with-timeout
>  If the user does not answer after SECONDS seconds, return DEFAULT-VALUE."
>    (with-timeout (seconds default-value)
>      (y-or-n-p prompt)))
> -
> +

Stop removing page breaks.


> -      FOR_EACH_TAIL (tail)
> +      FOR_EACH_TAIL_SAFE (tail)
>       {

...because?  Now you will no longer be able to quit from Fcopy_sequence,
and instead of a lock up upon encountering a circular list, you get the
list with random contents appended at the end.

> +/* Trigger any timers meeting their respective criteria.
>
> -/* Check whether a timer has fired.  To prevent larger problems we simply
> -   disregard elements that are not proper timers.  Do not make a circular
> -   timer list for the time being.
> +   For ordinary timers, this means current time is at
> +   or past their scheduled time.
>
> -   Returns the time to wait until the next timer fires.
> -   If no timer is active, return an invalid value.
> +   For idle timers, this means the idled period exceeds
> +   their idle threshold.
>
> -   As long as any timer is ripe, we run it.  */
> +   Return the time distance to the next upcoming timer.
> +*/
>
>  struct timespec
>  timer_check (void)
>  {
> -  struct timespec nexttime;
> -  Lisp_Object timers, idle_timers;
> -
> -  Lisp_Object tem = Vinhibit_quit;
> -  Vinhibit_quit = Qt;
> -  block_input ();
> -  turn_on_atimers (false);
> -
> -  /* 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.  */
> +  struct timespec now = current_timespec ();
> +  struct timespec idled = timespec_valid_p (timer_idleness_start_time)
> +    ? timespec_sub (now, timer_idleness_start_time)
> +    : invalid_timespec ();
> +  struct timespec until_next = invalid_timespec ();
> +  Lisp_Object *const lists[] = { &Vtimer_list, &Vtimer_idle_list };
> +  struct timespec const bogeys[] = { now, idled };
>
> -  /* Always consider the ordinary timers.  */
> -  timers = Fcopy_sequence (Vtimer_list);
> -  /* Consider the idle timers only if Emacs is idle.  */
> -  if (timespec_valid_p (timer_idleness_start_time))
> -    idle_timers = Fcopy_sequence (Vtimer_idle_list);
> -  else
> -    idle_timers = Qnil;
> -
> -  turn_on_atimers (true);
> -  unblock_input ();
> -  Vinhibit_quit = tem;
> -
> -  do
> +  for (int i = 0; i < 2; ++i)
>      {
> -      nexttime = timer_check_2 (timers, idle_timers);
> +      struct timespec bogey = bogeys[i];
> +      if (! timespec_valid_p (bogey))
> +     continue;
> +
> +      Lisp_Object timers = Fcopy_sequence (*lists[i]);
> +      FOR_EACH_TAIL_SAFE (timers)
> +     {
> +       struct timespec time;
> +       Lisp_Object *vec;
> +       CHECK_VECTOR (XCAR (timers));
> +       vec = XVECTOR (XCAR (timers))->contents;
> +       if (NILP (vec[0])) /* not yet triggered */
> +         {
> +           if (list4_to_timespec (vec[1], vec[2], vec[3], vec[8], &time))
> +             {
> +               /* Trigger when:
> +                  For ordinary timer, now is at or past trigger time.
> +                  For idle timer, idled duration at or past threshold.  */
> +               if (timespec_cmp (bogey, time) >= 0)
> +                 {
> +                   trigger_timer (XCAR (timers));
> +                 }
> +               else
> +                 {
> +                   struct timespec diff = timespec_sub (time, bogey);
> +                   if (! timespec_valid_p (until_next)
> +                       || timespectod (diff) < timespectod (until_next))
> +                     until_next = diff;
> +                 }
> +             }
> +         }
> +       else /* was triggered */
> +         {
> +           /* Clean up timers that errored out.  */
> +           if (NILP (vec[4])) /* if not repeated, delete it.  */
> +             *lists[i] = Fdelq (XCAR (timers), *lists[i]);
> +           else if (NILP (vec[7]) /* if not idle, reset it. */)
> +             vec[0] = Qnil;
> +         }
> +     }
>      }
> -  while (nexttime.tv_sec == 0 && nexttime.tv_nsec == 0);
>
> -  return nexttime;
> +  return until_next;
>  }

If you cannot quit your habit of making irrelevant changes that go
unexplained and reduce the total volume of the commentary, do not expect
anything you write to be installed.

Please file a bug for whatever you think is the problem instead.