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

[Pip Cet <pipcet@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 846 bytes --]

On Tue, Sep 1, 2015 at 5:19 PM, Eli Zaretskii <eliz@gnu.org> wrote:

> > The only code path that I see that's potentially dangerous is that
> atimers
> > appear to be executed even if input is blocked.
>
> Yes, that's exactly what bothered me.  Not calling QUIT prevents that.
>
> Alternatively, we could turn off atimers (by calling turn_on_atimers)
> while Fcopy_sequence runs.
>

I think that would be a better solution. I've done a quick grep for the
current atimers and at first glance they appear to be okay, but obviously
that's no guarantee for the future. It might be worth thinking about
block_input_and_atimers ().

I think it's safe to assume that Lisp timers are only checked if atimers
are enabled. 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.

[-- Attachment #1.2: Type: text/html, Size: 1301 bytes --]

[-- Attachment #2: 0001-Fix-potential-race-conditions-Bug-21380.patch --]
[-- Type: text/x-patch, Size: 1516 bytes --]

From 678bdba55e4a07e3baebad204c9fe5c55c99b3d3 Mon Sep 17 00:00:00 2001
From: Philip <pipcet@gmail.com>
Date: Tue, 1 Sep 2015 20:42:44 +0000
Subject: [PATCH] Fix potential race conditions (Bug#21380)

        * keyboard.c (timer_check): Call `block_input' and turn off
	atimers around the creation of the temporary timer list copy.

	* fns.c (concat): Don't assume argument size remains unchanged
	after call to `Fmake_list'.  Return incorrect results (but don't
	segfault) in that case.
---
 src/fns.c      | 3 +++
 src/keyboard.c | 4 ++++
 2 files changed, 7 insertions(+)

diff --git a/src/fns.c b/src/fns.c
index 26a98ab..15d9e64 100644
--- 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;
+
 		XSETCAR (tail, elt);
 		prev = tail;
 		tail = XCDR (tail);
diff --git a/src/keyboard.c b/src/keyboard.c
index dab32b1..4ce830d 100644
--- a/src/keyboard.c
+++ b/src/keyboard.c
@@ -4560,6 +4560,8 @@ timer_check (void)
 
   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
@@ -4573,6 +4575,8 @@ timer_check (void)
   else
     idle_timers = Qnil;
 
+  turn_on_atimers (true);
+  unblock_input ();
   Vinhibit_quit = tem;
 
   do
-- 
2.5.0