unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Noah Friedman <friedman@splode.com>
Subject: sit-for and idle timers
Date: Fri, 11 Aug 2006 12:48:41 -0700 (PDT)	[thread overview]
Message-ID: <20060811124841.840381.FMU5696@piglet.prv.splode.com> (raw)

The change to sit-for of 2006-07-26 ("Use new SECONDS arg of read-event
instead of a timer") seems to cause problems with idle-timers
which call sit-for.

The problem is that read-event's call tree ultimately results in calling
keyboard.c:read_char, which calls timer_start_idle.  This resets the
activation time for all the current idle timer events, which means that any
function on an idle timer which calls sit-for is now getting scheduled to
be run recursively if another interval of the appropriate length ensues.  I
imagine this can continue until max-lisp-eval-depth is reached.

Here's a small test case which demonstrates the problem:

    (defvar itimer-test-wait 0.5)
    (defvar itimer-test-depth 0)

    (defun itimer-test ()
      (setq itimer-test-depth (1+ itimer-test-depth))
      (unwind-protect
          (let ((flag nil))
            (while (sit-for itimer-test-wait)
              (setq flag (not flag))
              (message "itimer-test-depth: %-3d%s"
                       itimer-test-depth
                       (if flag " (blink)" ""))))
        (setq itimer-test-depth (1- itimer-test-depth))
        (message "itimer-test-depth: %d" itimer-test-depth)))

    (run-with-idle-timer 0.25 t 'itimer-test)

In the pre-7/26 implementation, this timer should never print a depth
greater than 1, and the "(blink)" text should blink on and off with a
regular rhythm.

In the post-7/26 implementation, itimer-test-depth increments indefinitely
until an event is read.

One solution to this problem is to bind timer-idle-list to nil while
calling read-event.  I tested this trivially with a small defadvice:

    (defadvice read-event (around idlefrob activate)
      (let ((timer-idle-list nil))
        ad-do-it))

I'm not sure if this (binding the variable in sit-for that is, not the use
of defadvice as a temporary kludge) is the correct solution since it will
preempt the activation of other idle timers.  But it's simple.  The less
simple solution is probably for sit-for to go back to using a timer itself.

Thoughts?

             reply	other threads:[~2006-08-11 19:48 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-08-11 19:48 Noah Friedman [this message]
2006-08-11 21:00 ` sit-for and idle timers Chong Yidong
2006-08-14 18:34 ` Chong Yidong
2006-08-14 19:20 ` Richard Stallman
2006-08-14 19:47   ` Chong Yidong
2006-08-14 20:05     ` Chong Yidong
2006-08-15 12:41       ` Richard Stallman
2006-08-15 20:12         ` Chong Yidong
2006-08-16 19:27           ` Richard Stallman
2006-08-16 19:38             ` Chong Yidong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20060811124841.840381.FMU5696@piglet.prv.splode.com \
    --to=friedman@splode.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).