bug#21380: 25.0.50; GTK-induced segfault when scheduling timer from window-configuration-change-hook

[Eli Zaretskii]

> Date: Wed, 2 Sep 2015 16:09:53 +0000
> From: Pip Cet <pipcet@gmail.com>
> Cc: 21380@debbugs.gnu.org
> 
>     > I think it's safe to assume that Lisp timers are only checked if atimers
>     are
>     > enabled.
>     
>     Those are two completely separate and independent features, so no,
>     it's not safe to make that assumption. Not sure why you need to
>     assume that, though.
> 
> So we can call turn_on_atimers (true) without potentially enabling atimers in 
> a
> critical section.

My confusion just grew a notch: one of these "atimers" is actually
Lisp timers, right?  If not, I'm afraid I don't see what you mean.

> My assumption was that the reason we have both Lisp timers and atimers is that
> atimers run strictly more often than Lisp timers.

They can be more accurate, but I see no reason why they should run
more often.

>     > If it isn't, I think the best way forward is to write
>     > block_input_and_atimers () and lock atimers with a counter just like
>     input is.
>     
>     Not sure I follow you. Are you saying that just calling block_input
>     followed by turn_on_atimers is somehow not enough to prevent some Lisp
>     from changing Vtimer_list under our feet?
>     
> 
> I'm not saying that, no, but if another function disables atimers, then runs
> Lisp timers, then does something critical that needs atimers to be disabled, 
> it
> might break.

We didn't need to disable atimers until now, except when manipulating
the atimers themselves.  The function we are discussing, which copies
Lisp timers, is the first one in need of this.  So I don't yet see the
need for a counter, but I don't object to one, either.

>     > --- a/src/fns.c
>     > +++ b/src/fns.c
>     > @@ -744,6 +744,9 @@ concat (ptrdiff_t nargs, Lisp_Object *args,
>     > /* Store this element into the result. */
>     > if (toindex < 0)
>     > {
>     > + if (NILP (tail))
>     > + break;
>     > +
>     
>     Is this part still needed?
> 
> As far as I know, the other two fixes are sufficient. It's needed in case
> someone calls copy_sequence on a list that's messed with by code run from a
> hook from QUIT, and merely succeeds in avoiding a segfault and producing
> incorrect results instead, so I'm not at all sure it should go in.

Right.