unofficial mirror of bug-gnu-emacs@gnu.org 
 help / color / mirror / code / Atom feed
* bug#9754: emacs -nv fails on glib 2.31
       [not found]           ` <1318609098.30586.7.camel@moonpix.lan>
@ 2011-10-14 17:48             ` Glenn Morris
  2011-10-16  5:15             ` Paul Eggert
                               ` (4 subsequent siblings)
  5 siblings, 0 replies; 31+ messages in thread
From: Glenn Morris @ 2011-10-14 17:48 UTC (permalink / raw)
  To: 9754; +Cc: Ryan Lortie


[ 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()





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

* bug#9754: emacs -nv fails on glib 2.31
       [not found]           ` <1318609098.30586.7.camel@moonpix.lan>
  2011-10-14 17:48             ` bug#9754: emacs -nv fails on glib 2.31 Glenn Morris
@ 2011-10-16  5:15             ` Paul Eggert
  2011-11-11 12:32             ` Olivier Blin
                               ` (3 subsequent siblings)
  5 siblings, 0 replies; 31+ messages in thread
From: Paul Eggert @ 2011-10-16  5:15 UTC (permalink / raw)
  To: Ryan Lortie; +Cc: 9754

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





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

* bug#9754: emacs -nv fails on glib 2.31
       [not found]           ` <1318609098.30586.7.camel@moonpix.lan>
  2011-10-14 17:48             ` bug#9754: emacs -nv fails on glib 2.31 Glenn Morris
  2011-10-16  5:15             ` Paul Eggert
@ 2011-11-11 12:32             ` Olivier Blin
  2011-11-11 17:16               ` Glenn Morris
  2012-01-28 21:14             ` Ulrich Mueller
                               ` (2 subsequent siblings)
  5 siblings, 1 reply; 31+ messages in thread
From: Olivier Blin @ 2011-11-11 12:32 UTC (permalink / raw)
  To: 9754

[-- Attachment #1: Type: text/plain, Size: 115 bytes --]

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


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: fix infinite loop in text mode select() when built with gtk support (emacs #9754, mga #3259) --]
[-- Type: text/x-patch, Size: 480 bytes --]

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

[-- Attachment #3: Type: text/plain, Size: 25 bytes --]

-- 
Olivier Blin - blino

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

* bug#9754: emacs -nv fails on glib 2.31
  2011-11-11 12:32             ` Olivier Blin
@ 2011-11-11 17:16               ` Glenn Morris
  0 siblings, 0 replies; 31+ messages in thread
From: Glenn Morris @ 2011-11-11 17:16 UTC (permalink / raw)
  To: Olivier Blin; +Cc: 9754

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.





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

* bug#9754: emacs -nv fails on glib 2.31
       [not found]           ` <1318609098.30586.7.camel@moonpix.lan>
                               ` (2 preceding siblings ...)
  2011-11-11 12:32             ` Olivier Blin
@ 2012-01-28 21:14             ` Ulrich Mueller
  2012-05-16 17:39             ` bug#9754: Issue with Emacs 23.4 Achim Gratz
  2012-05-17  6:24             ` Achim Gratz
  5 siblings, 0 replies; 31+ messages in thread
From: Ulrich Mueller @ 2012-01-28 21:14 UTC (permalink / raw)
  To: 9754

> 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. :-(





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

* bug#9754: Issue with Emacs 23.4
       [not found] ` <4FAC34DA.7040606@cs.ucla.edu>
@ 2012-05-15 19:08   ` Ken Brown
  2012-05-15 21:51     ` Paul Eggert
  0 siblings, 1 reply; 31+ messages in thread
From: Ken Brown @ 2012-05-15 19:08 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 9754, Douglas, William

[-- Attachment #1: Type: text/plain, Size: 755 bytes --]

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



[-- Attachment #2: xgselect.patch --]
[-- Type: text/plain, Size: 415 bytes --]

--- xgselect.c.orig	2012-05-15 13:53:13.000000000 -0400
+++ xgselect.c	2012-05-15 14:09:19.093750000 -0400
@@ -35,6 +35,9 @@
 xg_select (int max_fds, SELECT_TYPE *rfds, SELECT_TYPE *wfds, SELECT_TYPE *efds,
 	   EMACS_TIME *timeout)
 {
+  if (inhibit_window_system || !display_arg)
+    return select (max_fds, rfds, wfds, efds, timeout);
+
   SELECT_TYPE all_rfds, all_wfds;
   EMACS_TIME tmo, *tmop = timeout;
 


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

* bug#9754: Issue with Emacs 23.4
  2012-05-15 19:08   ` bug#9754: Issue with Emacs 23.4 Ken Brown
@ 2012-05-15 21:51     ` Paul Eggert
  2012-05-15 22:28       ` Ken Brown
  0 siblings, 1 reply; 31+ messages in thread
From: Paul Eggert @ 2012-05-15 21:51 UTC (permalink / raw)
  To: Ken Brown; +Cc: 9754, Douglas, William

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 {







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

* bug#9754: Issue with Emacs 23.4
  2012-05-15 21:51     ` Paul Eggert
@ 2012-05-15 22:28       ` Ken Brown
  2012-05-16  2:23         ` Stefan Monnier
  2012-05-16  2:28         ` Paul Eggert
  0 siblings, 2 replies; 31+ messages in thread
From: Ken Brown @ 2012-05-15 22:28 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 9754, Douglas, William

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






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

* bug#9754: Issue with Emacs 23.4
  2012-05-15 22:28       ` Ken Brown
@ 2012-05-16  2:23         ` Stefan Monnier
       [not found]           ` <1318609098.30586.7.camel@moonpix.lan>
  2012-05-16  6:42           ` Michael Albinus
  2012-05-16  2:28         ` Paul Eggert
  1 sibling, 2 replies; 31+ messages in thread
From: Stefan Monnier @ 2012-05-16  2:23 UTC (permalink / raw)
  To: Ken Brown; +Cc: 9754, Paul Eggert, Douglas, William

> 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





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

* bug#9754: Issue with Emacs 23.4
  2012-05-15 22:28       ` Ken Brown
  2012-05-16  2:23         ` Stefan Monnier
@ 2012-05-16  2:28         ` Paul Eggert
  1 sibling, 0 replies; 31+ messages in thread
From: Paul Eggert @ 2012-05-16  2:28 UTC (permalink / raw)
  To: Ken Brown; +Cc: 9754, Douglas, William

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.





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

* bug#9754: Issue with Emacs 23.4
  2012-05-16  2:23         ` Stefan Monnier
       [not found]           ` <1318609098.30586.7.camel@moonpix.lan>
@ 2012-05-16  6:42           ` Michael Albinus
  1 sibling, 0 replies; 31+ messages in thread
From: Michael Albinus @ 2012-05-16  6:42 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 9754, Paul Eggert, Douglas, William

Stefan Monnier <monnier@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.





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

* bug#9754: Issue with Emacs 23.4
       [not found]           ` <1318609098.30586.7.camel@moonpix.lan>
                               ` (3 preceding siblings ...)
  2012-01-28 21:14             ` Ulrich Mueller
@ 2012-05-16 17:39             ` Achim Gratz
  2012-05-17  1:19               ` Stefan Monnier
  2012-05-17  6:24             ` Achim Gratz
  5 siblings, 1 reply; 31+ messages in thread
From: Achim Gratz @ 2012-05-16 17:39 UTC (permalink / raw)
  To: 9754

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






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

* bug#9754: Issue with Emacs 23.4
  2012-05-16 17:39             ` bug#9754: Issue with Emacs 23.4 Achim Gratz
@ 2012-05-17  1:19               ` Stefan Monnier
  2012-05-17 11:04                 ` Ken Brown
  0 siblings, 1 reply; 31+ messages in thread
From: Stefan Monnier @ 2012-05-17  1:19 UTC (permalink / raw)
  To: Achim Gratz; +Cc: 9754

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





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

* bug#9754: Issue with Emacs 23.4
       [not found]           ` <1318609098.30586.7.camel@moonpix.lan>
                               ` (4 preceding siblings ...)
  2012-05-16 17:39             ` bug#9754: Issue with Emacs 23.4 Achim Gratz
@ 2012-05-17  6:24             ` Achim Gratz
  5 siblings, 0 replies; 31+ messages in thread
From: Achim Gratz @ 2012-05-17  6:24 UTC (permalink / raw)
  To: 9754

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






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

* bug#9754: Issue with Emacs 23.4
  2012-05-17  1:19               ` Stefan Monnier
@ 2012-05-17 11:04                 ` Ken Brown
  2012-05-17 12:43                   ` Ken Brown
  0 siblings, 1 reply; 31+ messages in thread
From: Ken Brown @ 2012-05-17 11:04 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 9754, Achim Gratz, Paul Eggert

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






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

* bug#9754: Issue with Emacs 23.4
  2012-05-17 11:04                 ` Ken Brown
@ 2012-05-17 12:43                   ` Ken Brown
  2012-05-17 12:49                     ` Andreas Schwab
  2012-05-17 15:02                     ` Stefan Monnier
  0 siblings, 2 replies; 31+ messages in thread
From: Ken Brown @ 2012-05-17 12:43 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 9754, Achim Gratz, Paul Eggert

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








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

* bug#9754: Issue with Emacs 23.4
  2012-05-17 12:43                   ` Ken Brown
@ 2012-05-17 12:49                     ` Andreas Schwab
  2012-05-17 13:20                       ` Ken Brown
  2012-05-17 15:02                     ` Stefan Monnier
  1 sibling, 1 reply; 31+ messages in thread
From: Andreas Schwab @ 2012-05-17 12:49 UTC (permalink / raw)
  To: Ken Brown; +Cc: 9754, Achim Gratz, Paul Eggert

Ken Brown <kbrown@cornell.edu> writes:

> +extern int x_in_use;

That should be put in a header.

Andreas.

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





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

* bug#9754: Issue with Emacs 23.4
  2012-05-17 12:49                     ` Andreas Schwab
@ 2012-05-17 13:20                       ` Ken Brown
  2012-05-17 14:01                         ` Andreas Schwab
  0 siblings, 1 reply; 31+ messages in thread
From: Ken Brown @ 2012-05-17 13:20 UTC (permalink / raw)
  To: Andreas Schwab; +Cc: 9754, Achim Gratz, Paul Eggert

On 5/17/2012 8:49 AM, Andreas Schwab wrote:
> Ken Brown<kbrown@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






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

* bug#9754: Issue with Emacs 23.4
  2012-05-17 13:20                       ` Ken Brown
@ 2012-05-17 14:01                         ` Andreas Schwab
  0 siblings, 0 replies; 31+ messages in thread
From: Andreas Schwab @ 2012-05-17 14:01 UTC (permalink / raw)
  To: Ken Brown; +Cc: 9754, Achim Gratz, Paul Eggert

Ken Brown <kbrown@cornell.edu> writes:

> On 5/17/2012 8:49 AM, Andreas Schwab wrote:
>> Ken Brown<kbrown@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@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."





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

* bug#9754: Issue with Emacs 23.4
  2012-05-17 12:43                   ` Ken Brown
  2012-05-17 12:49                     ` Andreas Schwab
@ 2012-05-17 15:02                     ` Stefan Monnier
  2012-05-17 18:26                       ` Ken Brown
  1 sibling, 1 reply; 31+ messages in thread
From: Stefan Monnier @ 2012-05-17 15:02 UTC (permalink / raw)
  To: Ken Brown; +Cc: 9754, Achim Gratz, Paul Eggert

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





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

* bug#9754: Issue with Emacs 23.4
  2012-05-17 15:02                     ` Stefan Monnier
@ 2012-05-17 18:26                       ` Ken Brown
  2012-05-18 16:33                         ` Glenn Morris
  2012-05-18 18:02                         ` Stefan Monnier
  0 siblings, 2 replies; 31+ messages in thread
From: Ken Brown @ 2012-05-17 18:26 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 9754, Achim Gratz, Paul Eggert



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





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

* bug#9754: Issue with Emacs 23.4
  2012-05-17 18:26                       ` Ken Brown
@ 2012-05-18 16:33                         ` Glenn Morris
  2012-05-18 17:09                           ` Ken Brown
  2012-05-18 18:02                         ` Stefan Monnier
  1 sibling, 1 reply; 31+ messages in thread
From: Glenn Morris @ 2012-05-18 16:33 UTC (permalink / raw)
  To: Ken Brown; +Cc: 9754, Achim Gratz, Paul Eggert

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?





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

* bug#9754: Issue with Emacs 23.4
  2012-05-18 16:33                         ` Glenn Morris
@ 2012-05-18 17:09                           ` Ken Brown
  2012-05-19  6:30                             ` Glenn Morris
  0 siblings, 1 reply; 31+ messages in thread
From: Ken Brown @ 2012-05-18 17:09 UTC (permalink / raw)
  To: Glenn Morris; +Cc: 9754, Achim Gratz, Paul Eggert

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





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

* bug#9754: Issue with Emacs 23.4
  2012-05-17 18:26                       ` Ken Brown
  2012-05-18 16:33                         ` Glenn Morris
@ 2012-05-18 18:02                         ` Stefan Monnier
  2012-05-18 18:38                           ` Ken Brown
  1 sibling, 1 reply; 31+ messages in thread
From: Stefan Monnier @ 2012-05-18 18:02 UTC (permalink / raw)
  To: Ken Brown; +Cc: 9754, Achim Gratz, Paul Eggert

> 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





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

* bug#9754: Issue with Emacs 23.4
  2012-05-18 18:02                         ` Stefan Monnier
@ 2012-05-18 18:38                           ` Ken Brown
  2012-05-18 19:12                             ` Paul Eggert
  0 siblings, 1 reply; 31+ messages in thread
From: Ken Brown @ 2012-05-18 18:38 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: 9754, Achim Gratz, Paul Eggert

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





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

* bug#9754: Issue with Emacs 23.4
  2012-05-18 18:38                           ` Ken Brown
@ 2012-05-18 19:12                             ` Paul Eggert
  2012-05-18 20:04                               ` Ken Brown
  0 siblings, 1 reply; 31+ messages in thread
From: Paul Eggert @ 2012-05-18 19:12 UTC (permalink / raw)
  To: Ken Brown; +Cc: 9754, Achim Gratz

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.





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

* bug#9754: Issue with Emacs 23.4
  2012-05-18 19:12                             ` Paul Eggert
@ 2012-05-18 20:04                               ` Ken Brown
  2012-05-19 21:56                                 ` Ken Brown
  0 siblings, 1 reply; 31+ messages in thread
From: Ken Brown @ 2012-05-18 20:04 UTC (permalink / raw)
  To: Paul Eggert; +Cc: 9754, Achim Gratz

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





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

* bug#9754: Issue with Emacs 23.4
  2012-05-18 17:09                           ` Ken Brown
@ 2012-05-19  6:30                             ` Glenn Morris
  2012-05-19 12:38                               ` Ken Brown
  0 siblings, 1 reply; 31+ messages in thread
From: Glenn Morris @ 2012-05-19  6:30 UTC (permalink / raw)
  To: Ken Brown; +Cc: 9754, Achim Gratz, Paul Eggert

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.





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

* bug#9754: Issue with Emacs 23.4
  2012-05-19  6:30                             ` Glenn Morris
@ 2012-05-19 12:38                               ` Ken Brown
  0 siblings, 0 replies; 31+ messages in thread
From: Ken Brown @ 2012-05-19 12:38 UTC (permalink / raw)
  To: Glenn Morris; +Cc: 9754, Achim Gratz, Paul Eggert

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





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

* bug#9754: Issue with Emacs 23.4
  2012-05-18 20:04                               ` Ken Brown
@ 2012-05-19 21:56                                 ` Ken Brown
  2012-05-25 20:32                                   ` Ken Brown
  0 siblings, 1 reply; 31+ messages in thread
From: Ken Brown @ 2012-05-19 21:56 UTC (permalink / raw)
  To: Paul Eggert; +Cc: Achim Gratz, 9754-done

Version: 24.2

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

Ken








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

* bug#9754: Issue with Emacs 23.4
  2012-05-19 21:56                                 ` Ken Brown
@ 2012-05-25 20:32                                   ` Ken Brown
  0 siblings, 0 replies; 31+ messages in thread
From: Ken Brown @ 2012-05-25 20:32 UTC (permalink / raw)
  To: 9754

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





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

end of thread, other threads:[~2012-05-25 20:32 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CABhPDt8f1iNs0+1N6Rn-8VbRPcscvbAda9KB388AzphWj4s_QA@mail.gmail.com>
     [not found] ` <4FAC34DA.7040606@cs.ucla.edu>
2012-05-15 19:08   ` bug#9754: Issue with Emacs 23.4 Ken Brown
2012-05-15 21:51     ` Paul Eggert
2012-05-15 22:28       ` Ken Brown
2012-05-16  2:23         ` Stefan Monnier
     [not found]           ` <1318609098.30586.7.camel@moonpix.lan>
2011-10-14 17:48             ` bug#9754: emacs -nv fails on glib 2.31 Glenn Morris
2011-10-16  5:15             ` Paul Eggert
2011-11-11 12:32             ` Olivier Blin
2011-11-11 17:16               ` Glenn Morris
2012-01-28 21:14             ` Ulrich Mueller
2012-05-16 17:39             ` bug#9754: Issue with Emacs 23.4 Achim Gratz
2012-05-17  1:19               ` Stefan Monnier
2012-05-17 11:04                 ` Ken Brown
2012-05-17 12:43                   ` Ken Brown
2012-05-17 12:49                     ` Andreas Schwab
2012-05-17 13:20                       ` Ken Brown
2012-05-17 14:01                         ` Andreas Schwab
2012-05-17 15:02                     ` Stefan Monnier
2012-05-17 18:26                       ` Ken Brown
2012-05-18 16:33                         ` Glenn Morris
2012-05-18 17:09                           ` Ken Brown
2012-05-19  6:30                             ` Glenn Morris
2012-05-19 12:38                               ` Ken Brown
2012-05-18 18:02                         ` Stefan Monnier
2012-05-18 18:38                           ` Ken Brown
2012-05-18 19:12                             ` Paul Eggert
2012-05-18 20:04                               ` Ken Brown
2012-05-19 21:56                                 ` Ken Brown
2012-05-25 20:32                                   ` Ken Brown
2012-05-17  6:24             ` Achim Gratz
2012-05-16  6:42           ` Michael Albinus
2012-05-16  2:28         ` Paul Eggert

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