GNU bug report logs - #9754
emacs -nv fails on glib 2.31

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 9754 in the body.
You can then email your comments to 9754 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Ryan Lortie <desrt <at> desrt.ca>
To: submit <at> debbugs.gnu.org
Subject: emacs -nv fails on glib 2.31
Date: Fri, 14 Oct 2011 12:18:18 -0400

Packages: emacs
Version: 23.3

When running "emacs -nw" with the latest glib version, emacs gets stuck
in this infinite loop:

in xg_select at /usr/src/debug/emacs-23.3/src/xgselect.c:59

58	        while (n_gfds > gfds_size) 
59	          gfds_size *= 2;

This code is buggy in the case that gfds_size is zero (since clearly, no
matter how many times you multiply by 2, you're not going to increase
it).

Further down in the same file, you see:

155	void
156	xgselect_initialize ()
157	{
158	#if defined (USE_GTK) || defined (HAVE_GCONF)
159	  gfds_size = 128;
160	  gfds = xmalloc (sizeof (*gfds)*gfds_size);


So it's clear that xgselect_initialize() is not being called in the
"-nw" case.  That makes sense -- why initialise GTK when not using it?

The problem is that xg_select() is used even in the "-nw" case, without
_initialize() having been called.  This worked before because an unused
GMainContext used to have 0 fds in it, so n_gfds would be zero and
gfds_size would not need to be increased, causing the bug to be skipped
over.

Recent changes in glib have introduced one fd to every GMainContext to
deal with the inherent race introduced by signal delivery (closing a
longstanding glib bug).  This means that the untouched GMainContext no
longer has 0 fds -- but 1.  This is what is triggering the problem in
the buggy code above.


The solution to this problem is one of:

 - ensure xgselect_initialize() is always called
 - don't use xg_select in -nw case
 - fix the code to deal with the array being zero-sized and nuke
   xgselect_initialize()

Message #8 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: 9754 <at> debbugs.gnu.org
Cc: Ryan Lortie <desrt <at> desrt.ca>
Subject: Re: bug#9754: emacs -nv fails on glib 2.31
Date: Fri, 14 Oct 2011 13:48:06 -0400

[ I am resending this report, which was assigned to the wrong package
due to a "Packages:"/"Package:" typo, so that it appears on the
bug-gnu-emacs list .]

From: Ryan Lortie
Packages: emacs
Version: 23.3

When running "emacs -nw" with the latest glib version, emacs gets stuck
in this infinite loop:

in xg_select at /usr/src/debug/emacs-23.3/src/xgselect.c:59

58	        while (n_gfds > gfds_size) 
59	          gfds_size *= 2;

This code is buggy in the case that gfds_size is zero (since clearly, no
matter how many times you multiply by 2, you're not going to increase
it).

Further down in the same file, you see:

155	void
156	xgselect_initialize ()
157	{
158	#if defined (USE_GTK) || defined (HAVE_GCONF)
159	  gfds_size = 128;
160	  gfds = xmalloc (sizeof (*gfds)*gfds_size);


So it's clear that xgselect_initialize() is not being called in the
"-nw" case.  That makes sense -- why initialise GTK when not using it?

The problem is that xg_select() is used even in the "-nw" case, without
_initialize() having been called.  This worked before because an unused
GMainContext used to have 0 fds in it, so n_gfds would be zero and
gfds_size would not need to be increased, causing the bug to be skipped
over.

Recent changes in glib have introduced one fd to every GMainContext to
deal with the inherent race introduced by signal delivery (closing a
longstanding glib bug).  This means that the untouched GMainContext no
longer has 0 fds -- but 1.  This is what is triggering the problem in
the buggy code above.


The solution to this problem is one of:

 - ensure xgselect_initialize() is always called
 - don't use xg_select in -nw case
 - fix the code to deal with the array being zero-sized and nuke
   xgselect_initialize()

Message #11 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Ryan Lortie <desrt <at> desrt.ca>
Cc: 9754 <at> debbugs.gnu.org
Subject: Re: emacs -nv fails on glib 2.31
Date: Sat, 15 Oct 2011 22:15:26 -0700

I don't think this bug can occur in the latest Emacs
since it uses xpalloc which should do the right
thing in this case.

Could you please double-check by trying the
latest Emacs pretest?  Thanks.

http://alpha.gnu.org/gnu/emacs/pretest/emacs-24.0.90.tar.gz

Message #14 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Olivier Blin <mageia <at> blino.org>
To: 9754 <at> debbugs.gnu.org
Subject: Re: emacs -nv fails on glib 2.31
Date: Fri, 11 Nov 2011 13:32:08 +0100

[Message part 1 (text/plain, inline)]

Attached is the patch I added in Mageia to fix this bug.
Something similar should be done in the emacs-23 branch.

[emacs-23.3-xgselect_init.patch (text/x-patch, inline)]

diff -up emacs-23.3/src/xgselect.c.xgselect_init emacs-23.3/src/xgselect.c
--- emacs-23.3/src/xgselect.c.xgselect_init	2011-01-08 18:45:14.000000000 +0100
+++ emacs-23.3/src/xgselect.c	2011-11-11 13:00:53.211765255 +0100
@@ -55,6 +55,9 @@ xg_select (max_fds, rfds, wfds, efds, ti
   do {
     if (n_gfds > gfds_size) 
       {
+        if (gfds_size == 0)
+          xgselect_initialize ();
+
         while (n_gfds > gfds_size) 
           gfds_size *= 2;
         xfree (gfds);

[Message part 3 (text/plain, inline)]

-- 
Olivier Blin - blino

Message #17 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Olivier Blin <mageia <at> blino.org>
Cc: 9754 <at> debbugs.gnu.org
Subject: Re: bug#9754: emacs -nv fails on glib 2.31
Date: Fri, 11 Nov 2011 12:16:05 -0500

Olivier Blin wrote:

> Attached is the patch I added in Mageia to fix this bug.
> Something similar should be done in the emacs-23 branch.

The Emacs 23 branch is no longer in use and won't be modified.
Could someone please test the latest 24.0.9* pretest, as was requested?

http://debbugs.gnu.org/cgi/bugreport.cgi?bug=9754#11

    I don't think this bug can occur in the latest Emacs since it uses
    xpalloc which should do the right thing in this case.

    Could you please double-check by trying the latest Emacs pretest?
    Thanks.

Message #22 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Ulrich Mueller <ulm <at> gentoo.org>
To: 9754 <at> debbugs.gnu.org
Subject: Re: emacs -nv fails on glib 2.31
Date: Sat, 28 Jan 2012 22:14:17 +0100

> The Emacs 23 branch is no longer in use and won't be modified.

Nice. Today Emacs 23.4 has been released from that "unused branch"
and is bound to fail with the next glib version. :-(

Message #31 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Ken Brown <kbrown <at> cornell.edu>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 9754 <at> debbugs.gnu.org, "Douglas, William" <william.douglas <at> intel.com>
Subject: Re: Issue with Emacs 23.4
Date: Tue, 15 May 2012 15:08:55 -0400

[Message part 1 (text/plain, inline)]

I'm resending this because I got back a message saying I had to 
unarchive bug 9754 before I could add to it.

On 5/10/2012 5:36 PM, Paul Eggert wrote:
> On 05/10/2012 02:25 PM, Douglas, William wrote:
>> Is there any chance of getting the fix for this applied or am I left
>> distro patching and waiting for 24?
>
> The latter -- at least, that's the current plan.

I think this bug still exists, in slightly different form, in emacs-24. 
  The call of g_main_context_query in xgselect.c:62 still uses the 
variables gfds and gfds_size, which are not initialized by 
xgselect_initialize if we're running emacs -nw.  But, more 
fundamentally, it doesn't make sense for emacs -nw to be interacting 
with GLib at all.  I suggest the attached patch.

Ken

[xgselect.patch (text/plain, attachment)]

Message #36 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Ken Brown <kbrown <at> cornell.edu>
Cc: 9754 <at> debbugs.gnu.org, "Douglas, William" <william.douglas <at> intel.com>
Subject: Re: Issue with Emacs 23.4
Date: Tue, 15 May 2012 14:51:53 -0700

On 05/15/2012 12:08 PM, Ken Brown wrote:

> I think this bug still exists, in slightly different form, in
> emacs-24.  The call of g_main_context_query in xgselect.c:62 still
> uses the variables gfds and gfds_size, which are not initialized by
> xgselect_initialize if we're running emacs -nw.

They are initialized to NULL and zero, which should work for
all their uses.

> more fundamentally, it doesn't make sense for emacs -nw to be interacting
> with GLib at all.  I suggest the attached patch.

That patch assumes C99's statements before declarations.
I assume the following minor rewrite of it is OK too?

=== modified file 'src/xgselect.c'
--- src/xgselect.c	2012-05-10 05:27:24 +0000
+++ src/xgselect.c	2012-05-15 21:30:36 +0000
@@ -38,17 +38,21 @@ xg_select (int max_fds, SELECT_TYPE *rfd
   SELECT_TYPE all_rfds, all_wfds;
   EMACS_TIME tmo, *tmop = timeout;
 
-  GMainContext *context = g_main_context_default ();
+  GMainContext *context;
   int have_wfds = wfds != NULL;
   int n_gfds = 0, our_tmo = 0, retval = 0, our_fds = 0;
   int i, nfds, fds_lim, tmo_in_millisec;
 
+  if (inhibit_window_system || !display_arg)
+    return select (max_fds, rfds, wfds, efds, timeout);
+
   if (rfds) memcpy (&all_rfds, rfds, sizeof (all_rfds));
   else FD_ZERO (&all_rfds);
   if (wfds) memcpy (&all_wfds, wfds, sizeof (all_rfds));
   else FD_ZERO (&all_wfds);
 
   /* Update event sources in GLib. */
+  context = g_main_context_default ();
   g_main_context_pending (context);
 
   do {

Message #39 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Ken Brown <kbrown <at> cornell.edu>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 9754 <at> debbugs.gnu.org, Chong Yidong <cyd <at> stupidchicken.com>, "Douglas,
	William" <william.douglas <at> intel.com>,
	Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Subject: Re: Issue with Emacs 23.4
Date: Tue, 15 May 2012 18:28:59 -0400

On 5/15/2012 5:51 PM, Paul Eggert wrote:
> On 05/15/2012 12:08 PM, Ken Brown wrote:
>
>> I think this bug still exists, in slightly different form, in
>> emacs-24.  The call of g_main_context_query in xgselect.c:62 still
>> uses the variables gfds and gfds_size, which are not initialized by
>> xgselect_initialize if we're running emacs -nw.
>
> They are initialized to NULL and zero, which should work for
> all their uses.

I don't know if we can be sure of this without digging into the various 
GLib functions that get called in the xg_select code.  I've been getting 
some crashes of emacs -nw on Cygwin after Cygwin's glib was updated to 
2.32 a few days ago.  That's what led me to start looking at xg_select.

But the point is moot if we're going to apply the patch we're discussing 
(which gets rid of the crashes).

>> more fundamentally, it doesn't make sense for emacs -nw to be interacting
>> with GLib at all.  I suggest the attached patch.
>
> That patch assumes C99's statements before declarations.
> I assume the following minor rewrite of it is OK too?

OK with me.  Should I go ahead and apply it to the trunk?  Or is it 
appropriate for the emacs-24 branch since it fixes a Cygwin crash?  I 
guess that's up to Stefan and Chong, so I've added them to the CC.

Ken

Message #42 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Ken Brown <kbrown <at> cornell.edu>
Cc: 9754 <at> debbugs.gnu.org, Chong Yidong <cyd <at> stupidchicken.com>,
	Paul Eggert <eggert <at> cs.ucla.edu>, "Douglas,
	William" <william.douglas <at> intel.com>
Subject: Re: Issue with Emacs 23.4
Date: Tue, 15 May 2012 22:23:09 -0400

> OK with me.  Should I go ahead and apply it to the trunk?  Or is it
> appropriate for the emacs-24 branch since it fixes a Cygwin crash?  I guess
> that's up to Stefan and Chong, so I've added them to the CC.

I don't understand the potential consequences of this patch, so I can't
really judge if it's appropriate for the emacs-24 branch.  At least it
doesn't seem "obviously correct" to me: what if the user opens up an
X frame after starting "emacs -nw"?  Are we sure we never need the Glib
loop (IIRC nowadays Glib is used not just for Gtk but also for other
things, like maybe D-Bus?).


        Stefan

Message #45 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Ken Brown <kbrown <at> cornell.edu>
Cc: 9754 <at> debbugs.gnu.org, Chong Yidong <cyd <at> stupidchicken.com>, "Douglas,
	William" <william.douglas <at> intel.com>,
	Stefan Monnier <monnier <at> IRO.UMontreal.CA>
Subject: Re: Issue with Emacs 23.4
Date: Tue, 15 May 2012 19:28:00 -0700

On 05/15/2012 03:28 PM, Ken Brown wrote:
> OK with me.  Should I go ahead and apply it to the trunk?
> Or is it appropriate for the emacs-24 branch since it fixes a Cygwin crash?

As I understand it, the bug is present in Emacs 23 and so
this is not a regression.  So I applied the change to the
trunk as bzr 108249.  I don't understand the situation
well enough to answer Stefan's questions, unfortunately --
which also suggests the patch isn't appropriate for the
emacs-24 branch.

Message #48 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 9754 <at> debbugs.gnu.org, Paul Eggert <eggert <at> cs.ucla.edu>, "Douglas,
	William" <william.douglas <at> intel.com>, Ken Brown <kbrown <at> cornell.edu>
Subject: Re: bug#9754: Issue with Emacs 23.4
Date: Wed, 16 May 2012 08:42:19 +0200

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

> (IIRC nowadays Glib is used not just for Gtk but also for other
> things, like maybe D-Bus?).

Emacs' dbusbind.c uses plain libdbus. No DBus-GLib, no GDBus.

>         Stefan

Best regards, Michael.

Message #51 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Achim Gratz <Stromeko <at> nexgo.de>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#9754: Issue with Emacs 23.4
Date: Wed, 16 May 2012 19:39:10 +0200

Stefan Monnier writes:
> At least it doesn't seem "obviously correct" to me: what if the user
> opens up an X frame after starting "emacs -nw"?

The hang arises when emacs is started without an X server present or at
least with DISPLAY not set.  I don't see how a user would be able to
open an X frame at a later point in time in that situation (or
reasonable expect he might be able to).


Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

Factory and User Sound Singles for Waldorf Blofeld:
http://Synth.Stromeko.net/Downloads.html#WaldorfSounds

Message #54 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Achim Gratz <Stromeko <at> nexgo.de>
Cc: 9754 <at> debbugs.gnu.org
Subject: Re: bug#9754: Issue with Emacs 23.4
Date: Wed, 16 May 2012 21:19:44 -0400

>> At least it doesn't seem "obviously correct" to me: what if the user
>> opens up an X frame after starting "emacs -nw"?
> The hang arises when emacs is started without an X server present or at
> least with DISPLAY not set.  I don't see how a user would be able to
> open an X frame at a later point in time in that situation (or
> reasonable expect he might be able to).

Easy: M-x make-frame-on-display.
Or start an emacs server and then connect to the server from an
emacsclient with a DISPLAY set (which ends up calling
make-frame-on-display as well, of course).


        Stefan

Message #57 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Achim Gratz <Stromeko <at> nexgo.de>
To: bug-gnu-emacs <at> gnu.org
Subject: Re: bug#9754: Issue with Emacs 23.4
Date: Thu, 17 May 2012 08:24:34 +0200

Stefan Monnier writes:
> Easy: M-x make-frame-on-display.
> Or start an emacs server and then connect to the server from an
> emacsclient with a DISPLAY set (which ends up calling
> make-frame-on-display as well, of course).

Didn't know that... but it's been a while since I used a multi-display X
setup.  Thanks.



Regards,
Achim.
-- 
+<[Q+ Matrix-12 WAVE#46+305 Neuron microQkb Andromeda XTk Blofeld]>+

SD adaptations for KORG EX-800 and Poly-800MkII V0.9:
http://Synth.Stromeko.net/Downloads.html#KorgSDada

Message #60 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Ken Brown <kbrown <at> cornell.edu>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 9754 <at> debbugs.gnu.org, Achim Gratz <Stromeko <at> nexgo.de>,
	Paul Eggert <eggert <at> cs.ucla.edu>
Subject: Re: bug#9754: Issue with Emacs 23.4
Date: Thu, 17 May 2012 07:04:13 -0400

On 5/16/2012 9:19 PM, Stefan Monnier wrote:
>>> At least it doesn't seem "obviously correct" to me: what if the user
>>> opens up an X frame after starting "emacs -nw"?
>> The hang arises when emacs is started without an X server present or at
>> least with DISPLAY not set.  I don't see how a user would be able to
>> open an X frame at a later point in time in that situation (or
>> reasonable expect he might be able to).
>
> Easy: M-x make-frame-on-display.
> Or start an emacs server and then connect to the server from an
> emacsclient with a DISPLAY set (which ends up calling
> make-frame-on-display as well, of course).

So maybe the test

  if (inhibit_window_system || !display_arg)

in my patch should be replaced by

  if (!x_in_use)

Ken

Message #63 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Ken Brown <kbrown <at> cornell.edu>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 9754 <at> debbugs.gnu.org, Achim Gratz <Stromeko <at> nexgo.de>,
	Paul Eggert <eggert <at> cs.ucla.edu>
Subject: Re: bug#9754: Issue with Emacs 23.4
Date: Thu, 17 May 2012 08:43:39 -0400

On 5/17/2012 7:04 AM, Ken Brown wrote:
> So maybe the test
>
> if (inhibit_window_system || !display_arg)
>
> in my patch should be replaced by
>
> if (!x_in_use)

Here's the patch (against the trunk) that goes along with my suggestion:

=== modified file 'src/xfns.c'
--- src/xfns.c  2012-05-02 10:20:35 +0000
+++ src/xfns.c  2012-05-17 12:29:53 +0000
@@ -136,7 +136,7 @@

 /* Nonzero if using X.  */

-static int x_in_use;
+int x_in_use;

 static Lisp_Object Qnone;
 static Lisp_Object Qsuppress_icon;

=== modified file 'src/xgselect.c'
--- src/xgselect.c      2012-05-16 02:22:53 +0000
+++ src/xgselect.c      2012-05-17 12:31:09 +0000
@@ -28,6 +28,8 @@
 #include <errno.h>
 #include <setjmp.h>

+extern int x_in_use;
+
 static GPollFD *gfds;
 static ptrdiff_t gfds_size;

@@ -43,7 +45,7 @@
   int n_gfds = 0, our_tmo = 0, retval = 0, our_fds = 0;
   int i, nfds, fds_lim, tmo_in_millisec;

-  if (inhibit_window_system || !display_arg)
+  if (!x_in_use)
     return select (max_fds, rfds, wfds, efds, timeout);

   if (rfds) memcpy (&all_rfds, rfds, sizeof (all_rfds));

Message #66 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> linux-m68k.org>
To: Ken Brown <kbrown <at> cornell.edu>
Cc: 9754 <at> debbugs.gnu.org, Achim Gratz <Stromeko <at> nexgo.de>,
	Stefan Monnier <monnier <at> iro.umontreal.ca>, Paul Eggert <eggert <at> cs.ucla.edu>
Subject: Re: bug#9754: Issue with Emacs 23.4
Date: Thu, 17 May 2012 14:49:18 +0200

Ken Brown <kbrown <at> cornell.edu> writes:

> +extern int x_in_use;

That should be put in a header.

Andreas.

-- 
Andreas Schwab, schwab <at> linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Message #69 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Ken Brown <kbrown <at> cornell.edu>
To: Andreas Schwab <schwab <at> linux-m68k.org>
Cc: 9754 <at> debbugs.gnu.org, Achim Gratz <Stromeko <at> nexgo.de>,
	Stefan Monnier <monnier <at> iro.umontreal.ca>, Paul Eggert <eggert <at> cs.ucla.edu>
Subject: Re: bug#9754: Issue with Emacs 23.4
Date: Thu, 17 May 2012 09:20:30 -0400

On 5/17/2012 8:49 AM, Andreas Schwab wrote:
> Ken Brown<kbrown <at> cornell.edu>  writes:
>
>> +extern int x_in_use;
>
> That should be put in a header.

Could you elaborate on that?  Are you suggesting a header xfns.h with 
(essentially) just that one line in it?

Thanks.

Ken

Message #72 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Andreas Schwab <schwab <at> linux-m68k.org>
To: Ken Brown <kbrown <at> cornell.edu>
Cc: 9754 <at> debbugs.gnu.org, Achim Gratz <Stromeko <at> nexgo.de>,
	Stefan Monnier <monnier <at> iro.umontreal.ca>, Paul Eggert <eggert <at> cs.ucla.edu>
Subject: Re: bug#9754: Issue with Emacs 23.4
Date: Thu, 17 May 2012 16:01:37 +0200

Ken Brown <kbrown <at> cornell.edu> writes:

> On 5/17/2012 8:49 AM, Andreas Schwab wrote:
>> Ken Brown<kbrown <at> cornell.edu>  writes:
>>
>>> +extern int x_in_use;
>>
>> That should be put in a header.
>
> Could you elaborate on that?  Are you suggesting a header xfns.h with
> (essentially) just that one line in it?

There are already a lot to choose from.

Andreas.

-- 
Andreas Schwab, schwab <at> linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Message #75 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Ken Brown <kbrown <at> cornell.edu>
Cc: 9754 <at> debbugs.gnu.org, Achim Gratz <Stromeko <at> nexgo.de>,
	Paul Eggert <eggert <at> cs.ucla.edu>
Subject: Re: bug#9754: Issue with Emacs 23.4
Date: Thu, 17 May 2012 11:02:11 -0400

>> So maybe the test
>> if (inhibit_window_system || !display_arg)
>> in my patch should be replaced by
>> if (!x_in_use)

Maybe.  As I said, I'm not familiar enough with that code to really know
what are the consequences, I just mentioned a few potential problems
(without even knowing whether they're real).

So unless someone more knowledgeable can confirm that the patch is
"obviously safe" I don't want to see it in the emacs-24 branch.


        Stefan

Message #78 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Ken Brown <kbrown <at> cornell.edu>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 9754 <at> debbugs.gnu.org, Achim Gratz <Stromeko <at> nexgo.de>,
	Paul Eggert <eggert <at> cs.ucla.edu>
Subject: Re: bug#9754: Issue with Emacs 23.4
Date: Thu, 17 May 2012 14:26:42 -0400

On 5/17/2012 11:02 AM, Stefan Monnier wrote:
>>> So maybe the test
>>> if (inhibit_window_system || !display_arg)
>>> in my patch should be replaced by
>>> if (!x_in_use)
>
> Maybe.  As I said, I'm not familiar enough with that code to really know
> what are the consequences, I just mentioned a few potential problems
> (without even knowing whether they're real).
>
> So unless someone more knowledgeable can confirm that the patch is
> "obviously safe" I don't want to see it in the emacs-24 branch.

I understand, and I'm no longer proposing the patch for the emacs-24 
branch.  My question is about what should be done in the trunk.  It 
seems to me that it would be better to use x_in_use, and I'm wondering 
if knowledgeable people agree.

Ken

Message #81 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Ken Brown <kbrown <at> cornell.edu>
Cc: 9754 <at> debbugs.gnu.org, Achim Gratz <Stromeko <at> nexgo.de>,
	Stefan Monnier <monnier <at> iro.umontreal.ca>, Paul Eggert <eggert <at> cs.ucla.edu>
Subject: Re: bug#9754: Issue with Emacs 23.4
Date: Fri, 18 May 2012 12:33:05 -0400

Ken Brown wrote:

> I understand, and I'm no longer proposing the patch for the emacs-24
> branch.  My question is about what should be done in the trunk.  It
> seems to me that it would be better to use x_in_use, and I'm wondering
> if knowledgeable people agree.

More knowledgeable people != me; but I just want to ask what the actual
issue is. You said:

  The call of g_main_context_query in xgselect.c:62 still uses the
  variables gfds and gfds_size, which are not initialized by
  xgselect_initialize if we're running emacs -nw. But, more
  fundamentally, it doesn't make sense for emacs -nw to be interacting
  with GLib at all.

This sort of makes it sound like the last sentence is a cosmetic issue,
and it was the first bit (uninitialized variables; although they
supposedly are initialized) that stopped your Cygwin crashes.

Is the crash reproducible on any other platform besides Cygwin?

Message #84 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Ken Brown <kbrown <at> cornell.edu>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 9754 <at> debbugs.gnu.org, Achim Gratz <Stromeko <at> nexgo.de>,
	Stefan Monnier <monnier <at> iro.umontreal.ca>, Paul Eggert <eggert <at> cs.ucla.edu>
Subject: Re: bug#9754: Issue with Emacs 23.4
Date: Fri, 18 May 2012 13:09:34 -0400

On 5/18/2012 12:33 PM, Glenn Morris wrote:
> Ken Brown wrote:
>
>> I understand, and I'm no longer proposing the patch for the emacs-24
>> branch.  My question is about what should be done in the trunk.  It
>> seems to me that it would be better to use x_in_use, and I'm wondering
>> if knowledgeable people agree.
>
> More knowledgeable people != me; but I just want to ask what the actual
> issue is. You said:
>
>    The call of g_main_context_query in xgselect.c:62 still uses the
>    variables gfds and gfds_size, which are not initialized by
>    xgselect_initialize if we're running emacs -nw. But, more
>    fundamentally, it doesn't make sense for emacs -nw to be interacting
>    with GLib at all.
>
> This sort of makes it sound like the last sentence is a cosmetic issue,
> and it was the first bit (uninitialized variables; although they
> supposedly are initialized) that stopped your Cygwin crashes.

No, I shouldn't have made the last sentence sound like a cosmetic issue. 
 Paul correctly pointed out that the variables actually are 
initialized, so I don't understand what caused the crashes.  They only 
started after a recent GLib upgrade, and I wasn't able to figure out 
why.  But in the course of debugging, I began to wonder why emacs -nw 
was calling GLib functions in the first place.  So I applied my patch 
and found that the crashes stopped.

> Is the crash reproducible on any other platform besides Cygwin?

I don't know.  It would be useful for someone to try it on a GNU/Linux 
system with glib >= 2.32.

Prior to the patch to xgselect.c, I was able to reliably reproduce the 
crash by simply starting emacs -nw and then doing C-x C-f C-g.  But the 
problem seemed to only occur when Cygwin was running on Windows XP or 
Windows Vista, not on Windows 7.  This makes it seem likely that the 
issue is Cygwin specific.

Ken

Message #87 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Ken Brown <kbrown <at> cornell.edu>
Cc: 9754 <at> debbugs.gnu.org, Achim Gratz <Stromeko <at> nexgo.de>,
	Paul Eggert <eggert <at> cs.ucla.edu>
Subject: Re: bug#9754: Issue with Emacs 23.4
Date: Fri, 18 May 2012 14:02:19 -0400

> I understand, and I'm no longer proposing the patch for the emacs-24 branch.
> My question is about what should be done in the trunk.  It seems to me that
> it would be better to use x_in_use, and I'm wondering if knowledgeable
> people agree.

I do think it's better, yes.


        Stefan

Message #90 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Ken Brown <kbrown <at> cornell.edu>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 9754 <at> debbugs.gnu.org, Achim Gratz <Stromeko <at> nexgo.de>,
	Paul Eggert <eggert <at> cs.ucla.edu>
Subject: Re: bug#9754: Issue with Emacs 23.4
Date: Fri, 18 May 2012 14:38:17 -0400

On 5/18/2012 2:02 PM, Stefan Monnier wrote:
>> I understand, and I'm no longer proposing the patch for the emacs-24 branch.
>> My question is about what should be done in the trunk.  It seems to me that
>> it would be better to use x_in_use, and I'm wondering if knowledgeable
>> people agree.
>
> I do think it's better, yes.

OK, thanks.  Then I'll probably apply the following patch, after waiting 
a few days to see how a new bug in xg_select that I just reported is 
resolved.

=== modified file 'src/lisp.h'
--- src/lisp.h  2012-05-09 17:51:30 +0000
+++ src/lisp.h  2012-05-18 13:38:36 +0000
@@ -3549,6 +3549,7 @@
 #ifdef HAVE_X_WINDOWS
 /* Defined in xfns.c */
 extern void syms_of_xfns (void);
+extern int x_in_use;

 /* Defined in xsmfns.c */
 extern void syms_of_xsmfns (void);

=== modified file 'src/xfns.c'
--- src/xfns.c  2012-05-02 10:20:35 +0000
+++ src/xfns.c  2012-05-18 18:34:10 +0000
@@ -136,7 +136,7 @@

 /* Nonzero if using X.  */

-static int x_in_use;
+int x_in_use;

 static Lisp_Object Qnone;
 static Lisp_Object Qsuppress_icon;

=== modified file 'src/xgselect.c'
--- src/xgselect.c      2012-05-16 02:22:53 +0000
+++ src/xgselect.c      2012-05-18 18:33:23 +0000
@@ -43,7 +43,7 @@
   int n_gfds = 0, our_tmo = 0, retval = 0, our_fds = 0;
   int i, nfds, fds_lim, tmo_in_millisec;

-  if (inhibit_window_system || !display_arg)
+  if (!x_in_use)
     return select (max_fds, rfds, wfds, efds, timeout);

   if (rfds) memcpy (&all_rfds, rfds, sizeof (all_rfds));


Ken

Message #93 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Ken Brown <kbrown <at> cornell.edu>
Cc: 9754 <at> debbugs.gnu.org, Achim Gratz <Stromeko <at> nexgo.de>,
	Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#9754: Issue with Emacs 23.4
Date: Fri, 18 May 2012 12:12:31 -0700

On 05/18/2012 11:38 AM, Ken Brown wrote:
> I'll probably apply the following patch

Thanks, that looks good, except please put the "extern int x_in_use;"
declaration in xterm.h not lisp.h, as x_in_use is related to
the X protocol as opposed to Lisp per se.  It's true that
lisp.h declares syms_of_xfns but there is some defense for
that as syms_of_xfns defines Lisp symbols; in contrast,
x_in_use has nothing to do with Lisp.

Message #96 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Ken Brown <kbrown <at> cornell.edu>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: 9754 <at> debbugs.gnu.org, Achim Gratz <Stromeko <at> nexgo.de>,
	Stefan Monnier <monnier <at> iro.umontreal.ca>
Subject: Re: bug#9754: Issue with Emacs 23.4
Date: Fri, 18 May 2012 16:04:15 -0400

On 5/18/2012 3:12 PM, Paul Eggert wrote:
> On 05/18/2012 11:38 AM, Ken Brown wrote:
>> I'll probably apply the following patch
>
> Thanks, that looks good, except please put the "extern int x_in_use;"
> declaration in xterm.h not lisp.h, as x_in_use is related to
> the X protocol as opposed to Lisp per se.  It's true that
> lisp.h declares syms_of_xfns but there is some defense for
> that as syms_of_xfns defines Lisp symbols; in contrast,
> x_in_use has nothing to do with Lisp.

OK.  I was naively putting the declaration in the same place as that of 
inhibit_window_system and display_arg, but your suggestion makes much 
more sense.

Ken

Message #99 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Glenn Morris <rgm <at> gnu.org>
To: Ken Brown <kbrown <at> cornell.edu>
Cc: 9754 <at> debbugs.gnu.org, Achim Gratz <Stromeko <at> nexgo.de>,
	Stefan Monnier <monnier <at> iro.umontreal.ca>, Paul Eggert <eggert <at> cs.ucla.edu>
Subject: Re: bug#9754: Issue with Emacs 23.4
Date: Sat, 19 May 2012 02:30:09 -0400

Ken Brown wrote:

> I don't know.  It would be useful for someone to try it on a GNU/Linux
> system with glib >= 2.32.
>
> Prior to the patch to xgselect.c, I was able to reliably reproduce the
> crash by simply starting emacs -nw and then doing C-x C-f C-g.

No such crash for me on Debian testing with glib 2.32 and a Gtk build of
the emacs-24 branch.

Message #102 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Ken Brown <kbrown <at> cornell.edu>
To: Glenn Morris <rgm <at> gnu.org>
Cc: 9754 <at> debbugs.gnu.org, Achim Gratz <Stromeko <at> nexgo.de>,
	Stefan Monnier <monnier <at> iro.umontreal.ca>, Paul Eggert <eggert <at> cs.ucla.edu>
Subject: Re: bug#9754: Issue with Emacs 23.4
Date: Sat, 19 May 2012 08:38:57 -0400

On 5/19/2012 2:30 AM, Glenn Morris wrote:
> Ken Brown wrote:
>
>> I don't know.  It would be useful for someone to try it on a GNU/Linux
>> system with glib>= 2.32.
>>
>> Prior to the patch to xgselect.c, I was able to reliably reproduce the
>> crash by simply starting emacs -nw and then doing C-x C-f C-g.
>
> No such crash for me on Debian testing with glib 2.32 and a Gtk build of
> the emacs-24 branch.

Thanks for testing.

Ken

Message #107 received at 9754-done <at> debbugs.gnu.org (full text, mbox):

From: Ken Brown <kbrown <at> cornell.edu>
To: Paul Eggert <eggert <at> cs.ucla.edu>
Cc: Achim Gratz <Stromeko <at> nexgo.de>, 9754-done <at> debbugs.gnu.org
Subject: Re: bug#9754: Issue with Emacs 23.4
Date: Sat, 19 May 2012 17:56:33 -0400

Version: 24.2

I've committed the change as bzr revision 108316, and I'm closing the bug.

Ken

Message #115 received at 9754 <at> debbugs.gnu.org (full text, mbox):

From: Ken Brown <kbrown <at> cornell.edu>
To: 9754 <at> debbugs.gnu.org
Subject: Re: bug#9754: Issue with Emacs 23.4
Date: Fri, 25 May 2012 16:32:17 -0400

Just to complete this discussion for the sake of the archives, the 
crashes I was getting resulted from a Cygwin bug, which has been fixed 
in the 2012-05-25 Cygwin snapshot.  But I still think that the patch I 
applied makes sense, as long as it doesn't cause other problems down the 
road.

Ken

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.