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

[Eli Zaretskii <eliz@gnu.org>]

> Date: Sun, 30 Aug 2015 20:56:33 +0000
> From: Pip Cet <pipcet@gmail.com>
> Cc: 21380@debbugs.gnu.org
> 
>     Then all we have to do is block QUIT during that copy, no?
> 
> Yes for this particular segfault.

Can you show a patch that fixes the original segfault in your use
case?  I'm afraid I lost track, what with all the different scenarios
and potential solutions being thrown at this.  We should install the
fix, assuming it's clean.

> No* for similar segfaults that I think pose equally severe problems: if any other function calls concat/copy-sequence on data that is modified by window-configuration-change-hook, it should* still be possible to produce the segfault.

Emacs gives you enough rope to hang yourself; there's nothing new
here.  We should strive to protect the Emacs internals so that they
won't cause segfaults, but in user code any bets are off, and "don't
do that" is a valid response to whoever does such things.

> So it wouldn't even be safe for window-configuration-change-hook to add a timer to the timer list, because the outer frame might be in the middle of creating a copy of the timer list for some Lisp code that hasn't blocked input. (As in my example below)

Futzing with timers from within some hooks is indeed fundamentally
dangerous.  But we should still try to minimize the probability of a
crash, especially when it's Emacs itself who makes the offending copy,
because people do dangerous things all the time, and expect them to
work.  In this case, blocking input should do, I think.

> I really don't think QUIT should run any Lisp hooks, to be honest.

I don't think this limitation could fly.  It will disable a lot of
useful use patterns, and the outcry will be loud and clear.

> If I'm wrong and QUIT should be able to run Lisp hooks, concat needs to be fixed not to rely on its argument's size being unchanged after the make_sequence call.

That can't do any harm, so let's do it, too.

> *-I have been able to verify this segfault by interrupting the program at just the right time and resizing its X window then:
>  - gdb --args emacs -Q
>  - evaluate the following code:
> 
> (run-with-timer 0 1 #'ignore) ;; so the timer list isn't empty
> (add-hook 'window-configuration-change-hook (lambda () (run-with-timer 0 nil #'ignore)))

> (while t
>   (copy-sequence timer-list)
>   (accept-process-output nil 0)
>   )
> 
>  - interrupt and set a breakpoint with "b Fmake_sequence if !input_blocked_p()".
>  - wait for breakpoint to be hit, verify that concat's args[0] is equal to Vtimer_list.
>  - resize the X window
>  - continue in gdb
>  - segfault
> 
> As far as I can tell, that should be reproducible. Also as far as I can tell, it's merely a matter of luck that an X resize doesn't happen at the point where I interrupted the program to artificially trigger the segfault. However, I admit that it is a separate issue, less likely to occur in practice, and I'll open another bug for it if that's the way you prefer things.

But if input is blocked, as it would be in the case of copying
timer-list inside timer_check, the X events will not be acted upon,
and the problem will not happen, right?

IOW, the above situation is a case of a user shooting herself in the
foot by having that particular function in the hook and that
particular code that copies timer-list (which is an internal variable
unwise users should not touch).  Am I right?