unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Should `cancel-timer' use `delete' instead of `delq'?
@ 2006-09-05  0:14 Drew Adams
  2006-09-05  1:44 ` Miles Bader
                   ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Drew Adams @ 2006-09-05  0:14 UTC (permalink / raw)


I may misunderstand this completely. If so, please set me straight. IIUC:

`cancel-timer' removes a particular vector (timer) from `timer-idle-list',
using delq. It does not remove all vectors that have the same elements,
however "same" might be defined for those elements. It uses eq for the
vector itself.

If you should happen to evaluate `run-with-idle-timer' more than once using
the same arguments, then a different timer (vector) would be created and run
for each evaluation. If you assigned the result of `run-with-idle-timer' to
a variable (setq or defvar), then passing that variable to `cancel-timer'
would cancel only the timer that was last created with those arguments to
`run-with-idle-timer' (and assigned to the variable).

I don't know - is that a feature or not?

Perhaps someone should never re-evaluate a particular `run-with-idle-timer'
expression, but it might happen, for instance, during debugging, if someone
used `C-M-x' on the timer defvar. Once that's done, IIUC, there is no way to
cancel the timer that was created and run previously (unless there is some
other variable that has it as value, or unless you manipulate
`timer-idle-list' directly and knowingly). I was bit by this, and I had to
delve into the source code to understand why my timer was still running.

E.g.

; 1st timer can never be cancelled, once 2nd is created.
(setq toto (run-with-idle-timer 10 t 'fn))
(setq toto (run-with-idle-timer 10 t 'fn))
; Cancel 2nd timer created and assigned to toto.
(cancel-timer toto)

I see code (e.g. in avoid.el) that uses setq in a context where it seems
like there could be multiple evaluations of the same `run-with-idle-timer'
expression. I don't know whether this particular code has a problem if it is
executed more than once; I just point it out to show that it might happen
that `run-with-idle-timer' is executed more than once with the same args,
and the same variable is assigned the result.

If the use of delq (vector identity) in `cancel-timer' is intentional and it
is considered a feature to have multiple idle timers with the same
parameters, then perhaps the doc should warn people of this gotcha - perhaps
advise them to use `cancel-timer' first, before `run-with-idle-timer':

(when(boundp 'foo-timer) (cancel-timer foo-timer))
(setq foo-timer (run-with-idle-timer 2 t 'foo))

I can only imagine that such a feature might have a use if, for some reason,
different variables were assigned to different timers that used the same
parameters:

(setq toto (run-with-idle-timer 10 t 'fn))
(setq titi (run-with-idle-timer 10 t 'fn))

I'm not sure when that might be useful, but it could be an argument to
support this feature.

If, on the other hand, the use of delq in `cancel-timer' is unintentional
and is not needed, then perhaps delete should be used instead of delq, so
that all timers with the same parameters are cancelled at once.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Should `cancel-timer' use `delete' instead of `delq'?
  2006-09-05  0:14 Should `cancel-timer' use `delete' instead of `delq'? Drew Adams
@ 2006-09-05  1:44 ` Miles Bader
  2006-09-05 15:38   ` Stefan Monnier
  2006-09-05 19:13 ` Stuart D. Herring
  2006-09-06  8:49 ` Richard Stallman
  2 siblings, 1 reply; 23+ messages in thread
From: Miles Bader @ 2006-09-05  1:44 UTC (permalink / raw)
  Cc: Emacs-Devel

It's almost certainly intentional that it uses delq.  A timer is a
unique entity.

If application code can create multiple timers and doesn't save all the
corresponding cookies for deletion, it's obviously a bug in that
application code.

-Miles
-- 
Come now, if we were really planning to harm you, would we be waiting here,
 beside the path, in the very darkest part of the forest?

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Should `cancel-timer' use `delete' instead of `delq'?
  2006-09-05  1:44 ` Miles Bader
@ 2006-09-05 15:38   ` Stefan Monnier
  2006-09-05 16:20     ` Drew Adams
                       ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Stefan Monnier @ 2006-09-05 15:38 UTC (permalink / raw)
  Cc: Drew Adams, Emacs-Devel

> It's almost certainly intentional that it uses delq.  A timer is a
> unique entity.

> If application code can create multiple timers and doesn't save all the
> corresponding cookies for deletion, it's obviously a bug in that
> application code.

While I somewhat agree with Miles, I also agree with Drew that it can lead
to undesirable situations where it's very difficult to return to a "normal"
session, other than by restarting Emacs.

I've been tempted to change timer-idle-list and timer-list so that they use
weak-pointers.  I.e. if you don't hold on to your timer by storing it in
a global var or somesuch, then it'll get cancelled/destroyed at the next GC.
I think this could work if we only do it for timers that use `repeat'.


        Stefan

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: Should `cancel-timer' use `delete' instead of `delq'?
  2006-09-05 15:38   ` Stefan Monnier
@ 2006-09-05 16:20     ` Drew Adams
  2006-09-05 17:22       ` Stefan Monnier
  2006-09-05 21:56     ` David Kastrup
  2006-09-06 19:05     ` Richard Stallman
  2 siblings, 1 reply; 23+ messages in thread
From: Drew Adams @ 2006-09-05 16:20 UTC (permalink / raw)


    >    If the use of delq (vector identity) in `cancel-timer'
    >    is intentional and it is considered a feature to have
    >    multiple idle timers with the same parameters, then
    >    perhaps the doc should warn people of this gotcha -
    >    perhaps advise them to use `cancel-timer' first, before
    >    `run-with-idle-timer':

    > It's almost certainly intentional that it uses delq.
    > A timer is a unique entity.

    > If application code can create multiple timers and doesn't
    > save all the corresponding cookies for deletion, it's obviously
    > a bug in that application code.

    While I somewhat agree with Miles, I also agree with Drew that
    it can lead to undesirable situations where it's very difficult
    to return to a "normal" session, other than by restarting Emacs.

    I've been tempted to change timer-idle-list and timer-list so
    that they use weak-pointers.  I.e. if you don't hold on to your
    timer by storing it in a global var or somesuch, then it'll get
    cancelled/destroyed at the next GC.
    I think this could work if we only do it for timers that use `repeat'.

Hard to agree with me ;-): I just asked a question - I didn't know what the
intention was.

What you suggest sounds good to me, though I'm pretty ignorant of this
stuff.

I think, though, that it still might make sense to document the gotcha; who
knows when the next GC will take place, and that could make the behavior
seem even more erratic (sometimes the timer is here, sometimes it's gone,
depending on a GC).

I don't know if this is a good way to handle the gotcha, but this is what
I've resorted to in my code (until I get a better suggestion):

(defvar foo-timer
  (progn ; Cancel to prevent ~duplication.
    (when (boundp 'foo-timer) (cancel-timer foo-timer))
    (run-with-idle-timer 2 t 'foo))
  "Timer used to foo whenever Emacs is idle.")

;; Turn it off, by default. You must use `toggle-foo' to turn it on.
(cancel-timer foo-timer)

Does it make sense to suggest doing something like this in Lisp code? Or at
least to point out that you probably don't want to eval
`run-with-idle-timer' without first cancelling a structurally equivalent
(but not eq) timer, if one already exists? Would we then also need to
explain that GC will eliminate any timers that are no longer held by
variables?

I'm not necessarily suggesting this change, but another possibility might be
to create and recommend (for most purposes) using a `define-idle-timer'
macro, instead of `run-with-idle-timer'. The macro would do more or less
what the above code does. That would ensure that the new timer was
associated with a variable and there were no zombie pseudo-duplicate timers
still running - each timer would have a unique name (associated variable).

The macro might even take an arg that determined whether the timer was to
start life running or cancelled. The macro would thus also let you
dissociate creation of the timer from activating it.

Further, the macro might, optionally, also create a toggle command that
would turn the timer on and off using `timer-activate-when-idle' and
`cancel-timer'. A la `define-minor-mode', with a message echoing the new
state and an internal variable `foo-activate-when-idle-p' that gets toggled.
That would be useful for some cases but not others, of course.

If we wanted to get more sophisticated, the macro could optionally take
additional Lisp code as input to include in each of the branches of the
toggle command. For example, if you wanted to add and remove a hook function
in the toggle-command branches, you could pass in the code to do that as
well.

People who wanted to use `run-with-idle-timer' directly could still do so,
of course, but most people would then use `define-idle-timer', which
prevents the gotcha.

Again, I'm no expert on timers. If this makes little sense, please ignore.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Should `cancel-timer' use `delete' instead of `delq'?
  2006-09-05 16:20     ` Drew Adams
@ 2006-09-05 17:22       ` Stefan Monnier
  2006-09-05 17:36         ` Drew Adams
  0 siblings, 1 reply; 23+ messages in thread
From: Stefan Monnier @ 2006-09-05 17:22 UTC (permalink / raw)
  Cc: Emacs-Devel

> (defvar foo-timer
>   (progn ; Cancel to prevent ~duplication.
>     (when (boundp 'foo-timer) (cancel-timer foo-timer))
>     (run-with-idle-timer 2 t 'foo))
>   "Timer used to foo whenever Emacs is idle.")

The traditional way to do something like the above is:

  (defvar foo-timer nil)

  (define-minor-mode foo
    "blala"
    :toto 1 :titi 0
    (when foo-timer
      (cancel foo-timer)
      (setq foo-timer nil))
    (when foo-mode
      (setq foo-timer (run-with-idle-timer 5 t 'foo-fun))))


-- Stefan

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: Should `cancel-timer' use `delete' instead of `delq'?
  2006-09-05 17:22       ` Stefan Monnier
@ 2006-09-05 17:36         ` Drew Adams
  2006-09-05 20:46           ` Kevin Rodgers
  0 siblings, 1 reply; 23+ messages in thread
From: Drew Adams @ 2006-09-05 17:36 UTC (permalink / raw)


    > (defvar foo-timer
    >   (progn ; Cancel to prevent ~duplication.
    >     (when (boundp 'foo-timer) (cancel-timer foo-timer))
    >     (run-with-idle-timer 2 t 'foo))
    >   "Timer used to foo whenever Emacs is idle.")

    The traditional way to do something like the above is:

      (defvar foo-timer nil)
      (define-minor-mode foo "blala" :toto 1 :titi 0
        (when foo-timer
          (cancel foo-timer)
          (setq foo-timer nil))
        (when foo-mode
          (setq foo-timer (run-with-idle-timer 5 t 'foo-fun))))

OK. I'm not sure why that's better, but it does seem to move a little toward
the direction I was suggesting with a `define-idle-timer' macro.

If we expect users to do this kind of thing (either my defvar or your
define-minor-mode), then don't we need to suggest it to them? IOW, wouldn't
it be worth documenting your "traditional" way? Better yet, wouldn't it be
worth creating a specific macro like `define-idle-timer' to do that for
them?

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Should `cancel-timer' use `delete' instead of `delq'?
  2006-09-05  0:14 Should `cancel-timer' use `delete' instead of `delq'? Drew Adams
  2006-09-05  1:44 ` Miles Bader
@ 2006-09-05 19:13 ` Stuart D. Herring
  2006-09-05 19:22   ` Drew Adams
  2006-09-06  8:49 ` Richard Stallman
  2 siblings, 1 reply; 23+ messages in thread
From: Stuart D. Herring @ 2006-09-05 19:13 UTC (permalink / raw)
  Cc: emacs-devel

> If, on the other hand, the use of delq in `cancel-timer' is unintentional
> and is not needed, then perhaps delete should be used instead of delq, so
> that all timers with the same parameters are cancelled at once.

Others have talked about the philosophy of timers, but let me point out
that this particular idea wouldn't work anyway.  Evaluate this to see why:

(let ((ts (list (run-with-timer 1.2 1 'ignore)
                (progn (sit-for 0.2) (run-with-timer 1.2 1 'ignore)))))
  (message "%s" ts)
  (prog1 (equal (car ts) (cadr ts))
    (mapcar 'cancel-timer ts)))

Because the precise timing of a timer (especially a repeating one) depends
on when it was created and how much time has since elapsed, the internal
representation of a timer cannot be merely its invocation parameters (and
in fact loses the delay argument, because a timer delayed by 10s started
9s ago looks the same as one delayed by 1s started now).  So "similar"
timers aren't necessarily `equal', and not all timers that are `equal'
were created with a call to `run-with-timer' with the same arguments. 
(Some may have been generated with `timer-create' directly!)  In other
words, we use `eq' because using `equal' doesn't work (although there are
obviously other implementations that could support such a practice).

Perhaps what you want is `cancel-function-timers'?  Certainly for testing
it's quite useful.  You could also just store some "mundane" value of
`timer-list' somewhere and restore it instead of specifically cancelling
anything.

Hope this helps,
Davis

-- 
This product is sold by volume, not by mass.  If it appears too dense or
too sparse, it is because mass-energy conversion has occurred during
shipping.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: Should `cancel-timer' use `delete' instead of `delq'?
  2006-09-05 19:13 ` Stuart D. Herring
@ 2006-09-05 19:22   ` Drew Adams
  0 siblings, 0 replies; 23+ messages in thread
From: Drew Adams @ 2006-09-05 19:22 UTC (permalink / raw)


    So "similar"
    timers aren't necessarily `equal', and not all timers that are `equal'
    were created with a call to `run-with-timer' with the same arguments.
    (Some may have been generated with `timer-create' directly!)  In other
    words, we use `eq' because using `equal' doesn't work (although
    there are
    obviously other implementations that could support such a practice).

    Perhaps what you want is `cancel-function-timers'?  Certainly
    for testing
    it's quite useful.  You could also just store some "mundane" value of
    `timer-list' somewhere and restore it instead of specifically cancelling
    anything.

Thanks for the info.

As I said, I have no problem with delq being used; I just wasn't certain of
the intention. Your post clarifies things.

The only "use case" I had in mind was what I sent before: the gotcha of
inadvertantly calling `run-with-idle-timer' more than once with the same
args, without thinking to cancel the previously launched timers.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Should `cancel-timer' use `delete' instead of `delq'?
  2006-09-05 17:36         ` Drew Adams
@ 2006-09-05 20:46           ` Kevin Rodgers
  2006-09-05 21:24             ` Drew Adams
  0 siblings, 1 reply; 23+ messages in thread
From: Kevin Rodgers @ 2006-09-05 20:46 UTC (permalink / raw)


Drew Adams wrote:
>     > (defvar foo-timer
>     >   (progn ; Cancel to prevent ~duplication.
>     >     (when (boundp 'foo-timer) (cancel-timer foo-timer))
>     >     (run-with-idle-timer 2 t 'foo))
>     >   "Timer used to foo whenever Emacs is idle.")
> 
>     The traditional way to do something like the above is:
> 
>       (defvar foo-timer nil)
>       (define-minor-mode foo "blala" :toto 1 :titi 0
>         (when foo-timer
>           (cancel foo-timer)
>           (setq foo-timer nil))
>         (when foo-mode
>           (setq foo-timer (run-with-idle-timer 5 t 'foo-fun))))
> 
> OK. I'm not sure why that's better, but it does seem to move a little toward
> the direction I was suggesting with a `define-idle-timer' macro.

It seems clearly better to me, because the (progn ...) form in your
defvar will only be evalated once: the first time, when foo-timer
is unbound.  Or am I missing something, in particular something that
would subsequently make foo-timer unbound?

-- 
Kevin

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: Should `cancel-timer' use `delete' instead of `delq'?
  2006-09-05 20:46           ` Kevin Rodgers
@ 2006-09-05 21:24             ` Drew Adams
  2006-09-06  1:11               ` Miles Bader
  0 siblings, 1 reply; 23+ messages in thread
From: Drew Adams @ 2006-09-05 21:24 UTC (permalink / raw)


    >     > (defvar foo-timer
    >     >   (progn ; Cancel to prevent ~duplication.
    >     >     (when (boundp 'foo-timer) (cancel-timer foo-timer))
    >     >     (run-with-idle-timer 2 t 'foo))
    >     >   "Timer used to foo whenever Emacs is idle.")
    >
    >     The traditional way to do something like the above is:
    >
    >       (defvar foo-timer nil)
    >       (define-minor-mode foo "blala" :toto 1 :titi 0
    >         (when foo-timer
    >           (cancel foo-timer)
    >           (setq foo-timer nil))
    >         (when foo-mode
    >           (setq foo-timer (run-with-idle-timer 5 t 'foo-fun))))
    >
    > OK. I'm not sure why that's better, but it does seem to move
    > a little toward the direction I was suggesting with a
    > `define-idle-timer' macro.

    It seems clearly better to me, because the (progn ...) form in your
    defvar will only be evalated once: the first time, when foo-timer
    is unbound.  Or am I missing something, in particular something that
    would subsequently make foo-timer unbound?

OK.

My point was about re-evaluating setq or defvar assignment of a variable to
a `run-with-idle-timer' expression. In the case of defvar, it would be by
hand, and I guess if someone knows enough to add the protective cruft, then
s?he should also know enough to cancel the timer before re-evaling the
defvar by hand. I was bitten by it, but then I might not be as thoughtful as
most users.

There are some uses of setq with `run-with-idle-timer' in the standard Lisp
libraries. I have no idea if there is any chance that that code could be
re-eval'd, leaving running, orphan timers as a result. Whether it's a minor
mode or a separate `define-idle-timer macro', providing the protection in
some kind of ready-to-use bottle seems like a good idea to me.

To repeat my question: Is it a good idea to either document the
"traditional" `define-minor-mode' approach or define a new macro for this,
or should we just let people discover this on their own?

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Should `cancel-timer' use `delete' instead of `delq'?
  2006-09-05 15:38   ` Stefan Monnier
  2006-09-05 16:20     ` Drew Adams
@ 2006-09-05 21:56     ` David Kastrup
  2006-09-06  0:59       ` Stefan Monnier
  2006-09-06 19:05     ` Richard Stallman
  2 siblings, 1 reply; 23+ messages in thread
From: David Kastrup @ 2006-09-05 21:56 UTC (permalink / raw)
  Cc: Emacs-Devel, Drew Adams, Miles Bader

Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> It's almost certainly intentional that it uses delq.  A timer is a
>> unique entity.
>
>> If application code can create multiple timers and doesn't save all the
>> corresponding cookies for deletion, it's obviously a bug in that
>> application code.
>
> While I somewhat agree with Miles, I also agree with Drew that it
> can lead to undesirable situations where it's very difficult to
> return to a "normal" session, other than by restarting Emacs.
>
> I've been tempted to change timer-idle-list and timer-list so that
> they use weak-pointers.  I.e. if you don't hold on to your timer by
> storing it in a global var or somesuch, then it'll get
> cancelled/destroyed at the next GC.  I think this could work if we
> only do it for timers that use `repeat'.

Does not sound too hot to me: after all, garbage collection happens
nondeterministically, and there are valid cases for repetitive timers
that will never need to be cancelled (a running clock somewhere, or a
yearly birthday reminder...).  It would surprise the user if his
timers suddenly stopped without reason when they ran perfectly well
for a while.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Should `cancel-timer' use `delete' instead of `delq'?
  2006-09-05 21:56     ` David Kastrup
@ 2006-09-06  0:59       ` Stefan Monnier
  0 siblings, 0 replies; 23+ messages in thread
From: Stefan Monnier @ 2006-09-06  0:59 UTC (permalink / raw)
  Cc: Emacs-Devel, Drew Adams, Miles Bader

> Does not sound too hot to me: after all, garbage collection happens
> nondeterministically, and there are valid cases for repetitive timers
> that will never need to be cancelled (a running clock somewhere, or a
> yearly birthday reminder...).  It would surprise the user if his
> timers suddenly stopped without reason when they ran perfectly well
> for a while.

Yes, just like reclamation of open-files shouldn't be done by the GC, this
should only be a debugging aid: it should warn about "lonely timers" rather
than kill them.


        Stefan

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Should `cancel-timer' use `delete' instead of `delq'?
  2006-09-05 21:24             ` Drew Adams
@ 2006-09-06  1:11               ` Miles Bader
  2006-09-06  2:09                 ` Drew Adams
  0 siblings, 1 reply; 23+ messages in thread
From: Miles Bader @ 2006-09-06  1:11 UTC (permalink / raw)
  Cc: emacs-devel

"Drew Adams" <drew.adams@oracle.com> writes:
> To repeat my question: Is it a good idea to either document the
> "traditional" `define-minor-mode' approach or define a new macro for this,
> or should we just let people discover this on their own?

I think the macro is inappropriate; the problem generally doesn't occur
in typical "define" contexts (for instance your defvar "protection" will
never get executed, because of the way defvar works).

If this is actually a problem in practice, just documenting it seems
good enough.

Is this potential problem really any more widespread than millions of
other very similar bugs though?  This is exactly the same as many other
sorts of resource allocation/deallocation; we don't explicitly warn
people about each of them because we assume than programmers know how to
handle this sort of thing in general.

-Miles

-- 
"Unless there are slaves to do the ugly, horrible, uninteresting work, culture
and contemplation become almost impossible. Human slavery is wrong, insecure,
and demoralizing.  On mechanical slavery, on the slavery of the machine, the
future of the world depends." -Oscar Wilde, "The Soul of Man Under Socialism"

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: Should `cancel-timer' use `delete' instead of `delq'?
  2006-09-06  1:11               ` Miles Bader
@ 2006-09-06  2:09                 ` Drew Adams
  2006-09-06  2:38                   ` Miles Bader
  2006-09-06  6:38                   ` David Kastrup
  0 siblings, 2 replies; 23+ messages in thread
From: Drew Adams @ 2006-09-06  2:09 UTC (permalink / raw)


    > To repeat my question: Is it a good idea to either document the
    > "traditional" `define-minor-mode' approach or define a new
    > macro for this, or should we just let people discover this on their
own?

    I think the macro is inappropriate; the problem generally doesn't occur
    in typical "define" contexts (for instance your defvar "protection" will
    never get executed, because of the way defvar works).

I was able to be bitten by it, using C-M-x on the defvar. And the general
point applies to setq as well.

I'd guess that most "define" contexts are not intended to create multiple
timers if for any reason they happen to get re-evaluated. And I'd guess that
most people creating those contexts will never even consider the potential
problem. I doubt that more than 1% of them have even heard of the
"traditional" `define-minor-mode' way of defining timers.

How many of them will be bit by this? Who knows? Why not help them avoid it?

    If this is actually a problem in practice, just documenting it seems
    good enough.

Documenting it is a good start.

To me, it makes sense to have a construct for defining a timer, which
doesn't also automatically activate it. Why not separate the two acts, as we
do (by default) with advice? As it stands now, to create a timer that is not
activated, you must create-and-activate it, and then cancel it.

    Is this potential problem really any more widespread than millions of
    other very similar bugs though?  This is exactly the same as many other
    sorts of resource allocation/deallocation;

Show us the millions of other very similar bugs, and we can compare ;-).

    we don't explicitly warn
    people about each of them because we assume than programmers know how to
    handle this sort of thing in general.

You assume too much, I think.

If I defvar or setq toto to (cons 1 2)), and I make no other assignments or
bindings to that cons cell, and then I setq toto to (cons 3 4), should I
assume that the first cons cell is still around causing mischief? Maybe, for
a while.

I think that one natural, if erroneous, expectation is that the orphan timer
will naturally go by the wayside at some point, and need not be thought much
about. There are other, just as natural, expectations that would be more
correct, but that expectation is a possible and reasonable one: don't worry
about that zombie timer; it's history; it may be toast already.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Should `cancel-timer' use `delete' instead of `delq'?
  2006-09-06  2:09                 ` Drew Adams
@ 2006-09-06  2:38                   ` Miles Bader
  2006-09-06  6:31                     ` Drew Adams
  2006-09-06  6:38                   ` David Kastrup
  1 sibling, 1 reply; 23+ messages in thread
From: Miles Bader @ 2006-09-06  2:38 UTC (permalink / raw)
  Cc: emacs-devel

"Drew Adams" <drew.adams@oracle.com> writes:
> I was able to be bitten by it, using C-M-x on the defvar.

That's a good point, but it's also a very special case -- if you're
using a debugging command to _force_ a construct to act in an abnormal
manner, you presumably know what your doing.

> And the general point applies to setq as well.

But I think the answer is not a macro, because there's an obvious, simple, and
extremely widespread idiom for dealing with this situation, which every
programmer should know about (if they don't, they're unlikely to realize they
should be using the special macro):

   (if var (free-non-gced-thing var))
   (setq var (make-non-gced-thing))

Not exactly rocket science...

> To me, it makes sense to have a construct for defining a timer, which
> doesn't also automatically activate it. Why not separate the two acts, as we
> do (by default) with advice? As it stands now, to create a timer that is not
> activated, you must create-and-activate it, and then cancel it.

Er, there _is_ such a function already:  `timer-create'.

But anyway, that doesn't help: the problem is when code "forgets" about
about an activated timer.  If you do (setq my-timer (timer-create)), and
the old value of my-timer was an activated timer, you're going to leak
the old timer even though the new timer is not activated.

>     Is this potential problem really any more widespread than millions of
>     other very similar bugs though?  This is exactly the same as many other
>     sorts of resource allocation/deallocation;
>
> Show us the millions of other very similar bugs, and we can compare ;-).

   static char *buf = 0;

   void init_my_package ()
   {
           buf = malloc (100);
           ...
   }

Whoops!

> I think that one natural, if erroneous, expectation is that the orphan timer
> will naturally go by the wayside at some point, and need not be thought much
> about. There are other, just as natural, expectations that would be more
> correct, but that expectation is a possible and reasonable one: don't worry
> about that zombie timer; it's history; it may be toast already.

Huh?  It doesn't seem reasonable or natural at all.  Activated timers
have an externally visible effect, so clearly they can't simply "go
away."

Someone writing code using timers needs to know this.

-miles

-- 
Come now, if we were really planning to harm you, would we be waiting here,
 beside the path, in the very darkest part of the forest?

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: Should `cancel-timer' use `delete' instead of `delq'?
  2006-09-06  2:38                   ` Miles Bader
@ 2006-09-06  6:31                     ` Drew Adams
  2006-09-06  6:48                       ` Miles Bader
                                         ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: Drew Adams @ 2006-09-06  6:31 UTC (permalink / raw)


    > I was able to be bitten by it, using C-M-x on the defvar.

    That's a good point, but it's also a very special case -- if you're
    using a debugging command to _force_ a construct to act in an abnormal
    manner, you presumably know what your doing.

Knowing what you're doing, when you think about it, does not always imply
thinking about what you're doing, when you do it. (Yogi Berra only wishes he
had said that.)

    > And the general point applies to setq as well.

    But I think the answer is not a macro, because there's an
    obvious, simple, and extremely widespread idiom for dealing
    with this situation, which every programmer should know
    about (if they don't, they're unlikely to realize they
    should be using the special macro):

       (if var (free-non-gced-thing var))
       (setq var (make-non-gced-thing))

    Not exactly rocket science...

Well, I think that's exactly what I used in my defvar (after having been
bitten once), so I could henceforth re-eval it ("abnormally") without worry
or forethought. In any case, the "traditional" way to do that is apparently
to use `define-minor-mode'...

The point, however, is that it's not that difficult for a programmer to not
think to do that (either one).

People often do learn to use recommended practices (e.g. defvar vs setq,
defcustom vs  defvar, define-minor-mode vs defun). I see no reason why
people wouldn't respond to a recommendation to (usually) use
`define-idle-timer' to define an idle timer. And the logic of the "obvious,
simple, and extremely widespread idiom" that I ultimately used but failed to
use initially, in spite of its patency, would be encapsulated in the macro.

    > To me, it makes sense to have a construct for defining a timer, which
    > doesn't also automatically activate it. Why not separate the
    > two acts, as we do (by default) with advice? As it stands now, to
    > create a timer that is not activated, you must create-and-activate
    > it, and then cancel it.

    Er, there _is_ such a function already: `timer-create'.

Oh, I see. Er, how could I be so ignorant and presumptuous? ;-)

And just how does one pass the necessary information to define an idle timer
to `timer-create'? Isn't it obvious? Here's the complete doc string for that
gem: "Create a timer object." And the complete definition of a timer object
is a length-8 vector. And of course `timer-create' is not mentioned once in
the Emacs-Lisp manual. But thanks for passing it on. Ah, yes, I see now,
`timer-set-idle-time'...

At the end of the day, I really don't care all that much about this. If you
want to help users by fixing up the doc a bit or adding a macro, fine. If
you don't, fine also. If someone brings it up again someday, you can tell
them about `timer-create'.

    But anyway, that doesn't help: the problem is when code "forgets" about
    about an activated timer.  If you do (setq my-timer (timer-create)), and
    the old value of my-timer was an activated timer, you're going to leak
    the old timer even though the new timer is not activated.

Yes, that was my first point, and it is a separate point. It's silly to say
that the one "doesn't help" the other; they are different points.

That the macro could also let you control when and whether to activate the
timer was a second point. I made other suggestions for the macro as well,
including (optionally) defining a toggling command. The separate points can
be examined separately.

    >     Is this potential problem really any more widespread than
    >     millions of other very similar bugs though?  This is exactly
    >     the same as many other sorts of resource allocation/deallocation;
    >
    > Show us the millions of other very similar bugs, and we can
    > compare ;-).

       static char *buf = 0;
       void init_my_package ()
       {buf = malloc (100);...}

    Whoops!

Nah; never bothers me while programming Lisp. But call it #1, anyway, if you
like; only 999,999 to go for your first million. You can throw all of C in
too as widespread similar bug #2, so only 999,998 to go for million #1...

    > I think that one natural, if erroneous, expectation is that
    > the orphan timer will naturally go by the wayside at some point,
    > and need not be thought much about. There are other, just as natural,
    > expectations that would be more correct, but that expectation is a
    > possible and reasonable one: don't worry about that zombie timer;
    > it's history; it may be toast already.

    Huh?  It doesn't seem reasonable or natural at all.

Not to a seasoned adept like yourself, no, of course not. But not all
Emacs-Lisp programmers reach your level.

    Activated timers have an externally visible effect, so clearly
    they can't simply "go away."

Clearly.

    Someone writing code using timers needs to know this.

You mean it's not "obvious, simple, and extremely widespread" knowledge
among all Emacs-Lisp programmers? You think maybe we ought to tell them?

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Should `cancel-timer' use `delete' instead of `delq'?
  2006-09-06  2:09                 ` Drew Adams
  2006-09-06  2:38                   ` Miles Bader
@ 2006-09-06  6:38                   ` David Kastrup
  1 sibling, 0 replies; 23+ messages in thread
From: David Kastrup @ 2006-09-06  6:38 UTC (permalink / raw)
  Cc: emacs-devel

"Drew Adams" <drew.adams@oracle.com> writes:

>     > To repeat my question: Is it a good idea to either document
>     > the "traditional" `define-minor-mode' approach or define a new
>     > macro for this, or should we just let people discover this on
>     > their own?
>
>     I think the macro is inappropriate; the problem generally
>     doesn't occur in typical "define" contexts (for instance your
>     defvar "protection" will never get executed, because of the way
>     defvar works).
>
> I was able to be bitten by it, using C-M-x on the defvar. And the
> general point applies to setq as well.

I don't think we need to worry about what damage a user might to
manually.  I really don't see this as a valid reason to stop multiple
timers from working.

> How many of them will be bit by this? Who knows? Why not help them
> avoid it?

We can't avoid people shooting themselves in the foot.  We might add
some command for killing timers in an emergency if really necessary.

>     we don't explicitly warn people about each of them because we
>     assume than programmers know how to handle this sort of thing in
>     general.
>
> You assume too much, I think.
>
> If I defvar or setq toto to (cons 1 2)), and I make no other
> assignments or bindings to that cons cell, and then I setq toto to
> (cons 3 4), should I assume that the first cons cell is still around
> causing mischief? Maybe, for a while.

A cons is not an active object.  You'll not notice the difference of
it being around or not if it is not referenced.  An active timer _is_
referenced.

> I think that one natural, if erroneous, expectation is that the
> orphan timer will naturally go by the wayside at some point, and
> need not be thought much about.

I don't see any such expectation as natural.

> There are other, just as natural, expectations that would be more
> correct, but that expectation is a possible and reasonable one:
> don't worry about that zombie timer;

An active timer is not a zombie.  Really.

> it's history; it may be toast already.

A timer stopping to work of his own volition is a bug.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Should `cancel-timer' use `delete' instead of `delq'?
  2006-09-06  6:31                     ` Drew Adams
@ 2006-09-06  6:48                       ` Miles Bader
  2006-09-06  7:29                       ` David Kastrup
  2006-09-06 14:00                       ` Stefan Monnier
  2 siblings, 0 replies; 23+ messages in thread
From: Miles Bader @ 2006-09-06  6:48 UTC (permalink / raw)
  Cc: emacs-devel

"Drew Adams" <drew.adams@oracle.com> writes:
>     Someone writing code using timers needs to know this.
>
> You mean it's not "obvious, simple, and extremely widespread" knowledge
> among all Emacs-Lisp programmers? You think maybe we ought to tell them?

The point is that it's a general fact that needs to be known about if
you're going to write code using timers (or any type of object that
requires some manual resource managment).  It's not some special case
related to run-with-idle-timer, so a macro as you suggested is not a
real solution.

So it's a general education issue; maybe the doc string/manual for
relevant functions (timer-activate, run-with-idle-timer, etc) could say something
like "timers activated using this function must always be explicitly
deactivated later; see info manual BLAHBLAH for more information."

-Miles

-- 
Is it true that nothing can be known?  If so how do we know this?  -Woody Allen

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Should `cancel-timer' use `delete' instead of `delq'?
  2006-09-06  6:31                     ` Drew Adams
  2006-09-06  6:48                       ` Miles Bader
@ 2006-09-06  7:29                       ` David Kastrup
  2006-09-06 14:00                       ` Stefan Monnier
  2 siblings, 0 replies; 23+ messages in thread
From: David Kastrup @ 2006-09-06  7:29 UTC (permalink / raw)
  Cc: emacs-devel

"Drew Adams" <drew.adams@oracle.com> writes:

>     > I was able to be bitten by it, using C-M-x on the defvar.
>
>     That's a good point, but it's also a very special case -- if you're
>     using a debugging command to _force_ a construct to act in an abnormal
>     manner, you presumably know what your doing.
>
> Knowing what you're doing, when you think about it, does not always
> imply thinking about what you're doing, when you do it.

When in doubt, Emacs should bite rather the people who don't know what
they are doing than those who do.

-- 
David Kastrup, Kriemhildstr. 15, 44793 Bochum

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Should `cancel-timer' use `delete' instead of `delq'?
  2006-09-05  0:14 Should `cancel-timer' use `delete' instead of `delq'? Drew Adams
  2006-09-05  1:44 ` Miles Bader
  2006-09-05 19:13 ` Stuart D. Herring
@ 2006-09-06  8:49 ` Richard Stallman
  2 siblings, 0 replies; 23+ messages in thread
From: Richard Stallman @ 2006-09-06  8:49 UTC (permalink / raw)
  Cc: emacs-devel

    If you should happen to evaluate `run-with-idle-timer' more than once using
    the same arguments, then a different timer (vector) would be created and run
    for each evaluation. If you assigned the result of `run-with-idle-timer' to
    a variable (setq or defvar), then passing that variable to `cancel-timer'
    would cancel only the timer that was last created with those arguments to
    `run-with-idle-timer' (and assigned to the variable).

    I don't know - is that a feature or not?

It is the way Lisp works.  If you want to remember more than one
object, and operate on them, you can keep them in a list.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Should `cancel-timer' use `delete' instead of `delq'?
  2006-09-06  6:31                     ` Drew Adams
  2006-09-06  6:48                       ` Miles Bader
  2006-09-06  7:29                       ` David Kastrup
@ 2006-09-06 14:00                       ` Stefan Monnier
  2006-09-06 15:27                         ` Drew Adams
  2 siblings, 1 reply; 23+ messages in thread
From: Stefan Monnier @ 2006-09-06 14:00 UTC (permalink / raw)
  Cc: emacs-devel

> Well, I think that's exactly what I used in my defvar (after having been
> bitten once), so I could henceforth re-eval it ("abnormally") without worry
> or forethought. In any case, the "traditional" way to do that is apparently
> to use `define-minor-mode'...

The traditional example I showed is not exempt from the OP.
When you re-evaluate the defvar with C-M-x, the timer var is forcefully
reset to nil, thus potentially forgetting a running timer, which then
becomes again difficult to stop.

Maybe you did realize that, but I got the feeling that you thought it didn't
suffer from this problem.

And the same thing could/would happen if you use `timer-create'.
The problem is not how you define/create your timer, but simply the fact
that if you "forget" your timer object, it'll still be active as long as
it's in the timer-list.

One way to "solve" this problem is to change C-M-x so that in the case the
variable's current value is a timer, it first cancels it, before resetting
the var.  Similarly to my "recent" patch which cancels timers when unloading
a package.


        Stefan

^ permalink raw reply	[flat|nested] 23+ messages in thread

* RE: Should `cancel-timer' use `delete' instead of `delq'?
  2006-09-06 14:00                       ` Stefan Monnier
@ 2006-09-06 15:27                         ` Drew Adams
  0 siblings, 0 replies; 23+ messages in thread
From: Drew Adams @ 2006-09-06 15:27 UTC (permalink / raw)


    > Well, I think that's exactly what I used in my defvar (after
    > having been bitten once), so I could henceforth re-eval it
    > ("abnormally") without worry or forethought. In any case, the
    > "traditional" way to do that is apparently to use
    > `define-minor-mode'...

    The traditional example I showed is not exempt from the OP.
    When you re-evaluate the defvar with C-M-x, the timer var is forcefully
    reset to nil, thus potentially forgetting a running timer, which then
    becomes again difficult to stop.

    Maybe you did realize that, but I got the feeling that you
    thought it didn't suffer from this problem.

I did realize that. I was attempting irony...

    And the same thing could/would happen if you use `timer-create'.
    The problem is not how you define/create your timer, but simply the fact
    that if you "forget" your timer object, it'll still be active as long as
    it's in the timer-list.

    One way to "solve" this problem is to change C-M-x so that in
    the case the variable's current value is a timer, it first cancels it,
    before resetting the var.  Similarly to my "recent" patch which cancels
    timers when unloading a package.

Yes. That's similar to what I proposed via a macro (to do the same thing).
If the first-cancel-then-define part is all that would be done, then your
suggestion is better. If more might be done (the activation and
toggle-command options I suggested), then a macro could be better.

Before this is laid to rest, let me request one more time that at least some
mention be made of this in the Elisp manual - in some form or other, whether
it's mentioned as a potential problem or whether some particular
practice/guideline is suggested/recommended.

If nothing else, it might be useful to point out that list `timer-idle-list'
is what controls whether an idle timer is active (similarly, `timer-list'),
so users will better understand that their defvar or whatever is not the
only pointer to the timer object. This may be obvious to some, but I'm sure
it is not obvious to some others.

IOW, even if `cancel-timer' needs to be handed a timer object (which you
might not have a handle for), you can always, as a last resort, delete your
timers by hand from `timer(-idle)-list', assuming that you can recognize
them (that is, you can reconstruct the list without them). Of course,
someone bitten by this pitfall and desperate to cancel timers will examine
the code of `cancel-timer' and find this out anyway, but foreknowledge of
this wouldn't hurt.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: Should `cancel-timer' use `delete' instead of `delq'?
  2006-09-05 15:38   ` Stefan Monnier
  2006-09-05 16:20     ` Drew Adams
  2006-09-05 21:56     ` David Kastrup
@ 2006-09-06 19:05     ` Richard Stallman
  2 siblings, 0 replies; 23+ messages in thread
From: Richard Stallman @ 2006-09-06 19:05 UTC (permalink / raw)
  Cc: emacs-devel, drew.adams, miles

    I've been tempted to change timer-idle-list and timer-list so that they use
    weak-pointers.

Please let's not discuss this now.

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2006-09-06 19:05 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-09-05  0:14 Should `cancel-timer' use `delete' instead of `delq'? Drew Adams
2006-09-05  1:44 ` Miles Bader
2006-09-05 15:38   ` Stefan Monnier
2006-09-05 16:20     ` Drew Adams
2006-09-05 17:22       ` Stefan Monnier
2006-09-05 17:36         ` Drew Adams
2006-09-05 20:46           ` Kevin Rodgers
2006-09-05 21:24             ` Drew Adams
2006-09-06  1:11               ` Miles Bader
2006-09-06  2:09                 ` Drew Adams
2006-09-06  2:38                   ` Miles Bader
2006-09-06  6:31                     ` Drew Adams
2006-09-06  6:48                       ` Miles Bader
2006-09-06  7:29                       ` David Kastrup
2006-09-06 14:00                       ` Stefan Monnier
2006-09-06 15:27                         ` Drew Adams
2006-09-06  6:38                   ` David Kastrup
2006-09-05 21:56     ` David Kastrup
2006-09-06  0:59       ` Stefan Monnier
2006-09-06 19:05     ` Richard Stallman
2006-09-05 19:13 ` Stuart D. Herring
2006-09-05 19:22   ` Drew Adams
2006-09-06  8:49 ` Richard Stallman

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).