Debian Bug report logs - #508126
x11-utils: xprop -spy does not handle destruction properly

Toggle useless messages

Message #5 received at submit@bugs.debian.org (full text, mbox, reply):

From: Ben Hutchings <ben@decadent.org.uk>

To: Debian Bug Tracking System <submit@bugs.debian.org>

Subject: x11-utils: xprop -spy does not handle destruction properly

Date: Mon, 08 Dec 2008 05:08:04 +0000

Package: x11-utils
Version: 7.3+2
Severity: important

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I would expect xprop -spy to exit cleanly when the target window is
destroyed.  The actual behaviour is that it sometimes dies with a
BadWindow error and sometimes hangs around after the target has been
destroyed.  This causes problems for xdg-screensaver, which relies
on it to exit when the target is destroyed.

Ben.

- -- System Information:
Debian Release: 5.0
  APT prefers unstable
  APT policy: (500, 'unstable'), (500, 'testing'), (100, 'experimental')
Architecture: i386 (i686)

Kernel: Linux 2.6.26-1-686 (SMP w/1 CPU core)
Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash

Versions of packages x11-utils depends on:
ii  cpp                    4:4.3.2-2         The GNU C preprocessor (cpp)
ii  libc6                  2.7-16            GNU C Library: Shared libraries
ii  libfontconfig1         2.6.0-3           generic font configuration library
ii  libfontenc1            1:1.0.4-3         X11 font encoding library
ii  libfreetype6           2.3.7-2           FreeType 2 font engine, shared lib
ii  libgl1-mesa-glx [libgl 7.0.3-6           A free implementation of the OpenG
ii  libice6                2:1.0.4-1         X11 Inter-Client Exchange library
ii  libsm6                 2:1.0.3-2         X11 Session Management library
ii  libx11-6               2:1.1.5-2         X11 client-side library
ii  libxaw7                2:1.0.4-2         X11 Athena Widget library
ii  libxext6               2:1.0.4-1         X11 miscellaneous extension librar
ii  libxft2                2.1.12-3          FreeType-based font drawing librar
ii  libxi6                 2:1.1.4-1         X11 Input extension library
ii  libxinerama1           2:1.0.3-2         X11 Xinerama extension library
ii  libxmu6                2:1.0.4-1         X11 miscellaneous utility library
ii  libxmuu1               2:1.0.4-1         X11 miscellaneous micro-utility li
ii  libxrender1            1:0.9.4-2         X Rendering Extension client libra
ii  libxt6                 1:1.0.5-3         X11 toolkit intrinsics library
ii  libxtst6               2:1.0.3-1         X11 Testing -- Resource extension 
ii  libxv1                 2:1.0.4-1         X11 Video extension library
ii  libxxf86dga1           2:1.0.2-1         X11 Direct Graphics Access extensi
ii  libxxf86vm1            1:1.0.2-1         X11 XFree86 video mode extension l
ii  x11-common             1:7.3+18          X Window System (X.Org) infrastruc
ii  zlib1g                 1:1.2.3.3.dfsg-12 compression library - runtime

x11-utils recommends no packages.

Versions of packages x11-utils suggests:
ii  mesa-utils                    7.0.3-6    Miscellaneous Mesa GL utilities

- -- no debconf information

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iD8DBQFJPKu079ZNCRIGYgcRArrAAJ9+Q/XMiifbNJ+faDU1ir0IEa0yFwCfeUI+
jdkCoYzWKur9YQ/M5Zw+F5Q=
=v+q5
-----END PGP SIGNATURE-----

Message #10 received at 508126@bugs.debian.org (full text, mbox, reply):

From: Ben Hutchings <ben@decadent.org.uk>

To: 508126@bugs.debian.org

Subject: Re: x11-utils: xprop -spy does not handle destruction properly

Date: Wed, 24 Dec 2008 23:37:04 +0000

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

This patch seems to do the job.  Firstly we listen for destroy events.
However, since window destruction is asynchronous with other clients, we
may receive a BadWindow (or BadMatch, due to id reuse) error while
reporting a property change.  So secondly we catch these errors.
Possibly we should print a new-line before exiting in this case, since
the error received when we have generated partial output for a property
change.

Ben.

diff -Nru x11-utils.orig/xprop/xprop.c x11-utils/xprop/xprop.c
--- x11-utils.orig/xprop/xprop.c
+++ x11-utils/xprop/xprop.c
@@ -1596,6 +1596,18 @@
 
 static int spy = 0;
 
+static int (*old_error_handler)(Display *dpy, XErrorEvent *ev);
+
+static int spy_error_handler(Display *dpy, XErrorEvent *ev)
+{
+    if (ev->error_code == BadWindow || ev->error_code == BadMatch) {
+	/* Window was destroyed */
+	exit(0);
+    }
+
+    return old_error_handler(dpy, ev);
+}
+
 int
 main (int argc, char **argv)
 {
@@ -1738,9 +1750,14 @@
 	XEvent event;
 	const char *format, *dformat;
 	
-	XSelectInput(dpy, target_win, PropertyChangeMask);
+	XSelectInput(dpy, target_win, PropertyChangeMask | StructureNotifyMask);
+	old_error_handler = XSetErrorHandler(spy_error_handler);
 	for (;;) {
 	    XNextEvent(dpy, &event);
+	    if (event.type == DestroyNotify)
+		break;
+	    if (event.type != PropertyNotify)
+		continue;
 	    format = dformat = NULL;
 	    if (props) {
 		int i;
--- END ---

-- 
Ben Hutchings
All the simple programs have been written, and all the good names taken.

[signature.asc (application/pgp-signature, inline)]

Message #19 received at 508126@bugs.debian.org (full text, mbox, reply):

From: Ben Hutchings <ben@decadent.org.uk>

To: debian-release@lists.debian.org

Cc: 374644@bugs.debian.org, 508125@bugs.debian.org, 508126@bugs.debian.org

Subject: Re: xine-ui: ctrl/shift key press emulation implementation broken

Date: Fri, 02 Jan 2009 15:30:01 +0000

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

I need some guidance on how to deal with the following related bugs:

#374644: xine-ui: ctrl/shift key press emulation implementation broken
#506001: xine-ui: xine causes left ctrl keyup events every 20 seconds
These are the same bug: xine-ui suppresses screensavers by injecting
fake keystrokes, which may be received by other windows.  It also
suppresses gnome-screensaver cleanly, so we could disable the key
injection code without affecting GNOME users.  However, this would be a
regression for users of the X server screensaver, xscreensaver or the
KDE screensaver.

My proposed fix involves using xdg-screensaver, which supports all the
different screensavers.  However, this suffers from the following bugs:
#508125: xdg-screensaver: Race in suspend/resume can lead to process leak
#508126: x11-utils: xprop -spy does not handle destruction properly
I do not think that these are, in themselves, RC.

I have now proposed fixes for all of these, and the result appears to be
robust.  Should I NMU with these fixes?  Should any of these bugs be
down/upgraded?

Ben.

-- 
Ben Hutchings
Lowery's Law:
             If it jams, force it. If it breaks, it needed replacing anyway.

[signature.asc (application/pgp-signature, inline)]

Message #24 received at 508126@bugs.debian.org (full text, mbox, reply):

From: Per Olofsson <pelle@dsv.su.se>

To: Ben Hutchings <ben@decadent.org.uk>, 508125@bugs.debian.org

Cc: debian-release@lists.debian.org, 374644@bugs.debian.org, 508126@bugs.debian.org

Subject: Re: Bug#508125: xine-ui: ctrl/shift key press emulation implementation broken

Date: Fri, 02 Jan 2009 18:17:41 +0100

Hi Ben,

Ben Hutchings wrote:
> I need some guidance on how to deal with the following related bugs:
> 
> #374644: xine-ui: ctrl/shift key press emulation implementation broken
> #506001: xine-ui: xine causes left ctrl keyup events every 20 seconds
> These are the same bug: xine-ui suppresses screensavers by injecting
> fake keystrokes, which may be received by other windows.  It also
> suppresses gnome-screensaver cleanly, so we could disable the key
> injection code without affecting GNOME users.  However, this would be a
> regression for users of the X server screensaver, xscreensaver or the
> KDE screensaver.
> 
> My proposed fix involves using xdg-screensaver, which supports all the
> different screensavers.  However, this suffers from the following bugs:
> #508125: xdg-screensaver: Race in suspend/resume can lead to process leak
> #508126: x11-utils: xprop -spy does not handle destruction properly
> I do not think that these are, in themselves, RC.
>
> I have now proposed fixes for all of these, and the result appears to be
> robust.  Should I NMU with these fixes?  Should any of these bugs be
> down/upgraded?

Feel free to NMU xdg-utils, as the maintainer I have no problems with
that. I would appreciate if you commit directly to the git repository,
which is in collab-maint (but it's not necessary).

Sorry for not responding earlier.

-- 
Pelle

Message #29 received at 508126@bugs.debian.org (full text, mbox, reply):

From: Julien Cristau <jcristau@debian.org>

To: Ben Hutchings <ben@decadent.org.uk>, 508126@bugs.debian.org

Cc: debian-release@lists.debian.org

Subject: Re: Bug#508126: xine-ui: ctrl/shift key press emulation implementation broken

Date: Sat, 3 Jan 2009 18:00:06 +0100

On Fri, Jan  2, 2009 at 15:30:01 +0000, Ben Hutchings wrote:

> #508126: x11-utils: xprop -spy does not handle destruction properly
> 
the xprop patch looks reasonable afaict, so go ahead and NMU.  somebody
should also forward that patch to bugs.freedesktop.org.

Thanks,
Julien

Message #34 received at 508126-close@bugs.debian.org (full text, mbox, reply):

From: Ben Hutchings <ben@decadent.org.uk>

To: 508126-close@bugs.debian.org

Subject: Bug#508126: fixed in x11-utils 7.3+2+nmu1

Date: Sat, 03 Jan 2009 23:02:07 +0000

Source: x11-utils
Source-Version: 7.3+2+nmu1

We believe that the bug you reported is fixed in the latest version of
x11-utils, which is due to be installed in the Debian FTP archive:

x11-utils_7.3+2+nmu1.dsc
  to pool/main/x/x11-utils/x11-utils_7.3+2+nmu1.dsc
x11-utils_7.3+2+nmu1.tar.gz
  to pool/main/x/x11-utils/x11-utils_7.3+2+nmu1.tar.gz
x11-utils_7.3+2+nmu1_i386.deb
  to pool/main/x/x11-utils/x11-utils_7.3+2+nmu1_i386.deb



A summary of the changes between this version and the previous one is
attached.

Thank you for reporting the bug, which will now be closed.  If you
have further comments please address them to 508126@bugs.debian.org,
and the maintainer will reopen the bug report if appropriate.

Debian distribution maintenance software
pp.
Ben Hutchings <ben@decadent.org.uk> (supplier of updated x11-utils package)

(This message was generated automatically at their request; if you
believe that there is a problem with it please contact the archive
administrators by mailing ftpmaster@debian.org)


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Format: 1.8
Date: Sat, 03 Jan 2009 22:22:40 +0000
Source: x11-utils
Binary: x11-utils
Architecture: source i386
Version: 7.3+2+nmu1
Distribution: unstable
Urgency: low
Maintainer: Debian X Strike Force <debian-x@lists.debian.org>
Changed-By: Ben Hutchings <ben@decadent.org.uk>
Description: 
 x11-utils  - X11 utilities
Closes: 508126
Changes: 
 x11-utils (7.3+2+nmu1) unstable; urgency=low
 .
   * Non-maintainer upload.
   * Make xprop -spy exit cleanly when target window is destroyed
     (closes: #508126)
Checksums-Sha1: 
 91d530375bf88db8fbb8607538b3a72975ed160b 1432 x11-utils_7.3+2+nmu1.dsc
 8c1f2bee4ac70ee8afaa5719422c80294bdae153 2134376 x11-utils_7.3+2+nmu1.tar.gz
 be24f37125b662b4ff54279a199112138543e998 202558 x11-utils_7.3+2+nmu1_i386.deb
Checksums-Sha256: 
 0187cc0cc3a903280dff0b60b5c18319de25a3ed35176987865579fb23c649e4 1432 x11-utils_7.3+2+nmu1.dsc
 d3134f36600f7a7d8179134514765aa2f35a8d9b86da12160acd2beece14d09f 2134376 x11-utils_7.3+2+nmu1.tar.gz
 4f96e376be351370ec980e2fb04e5267aa5b697c74016d689378d11ae1bba575 202558 x11-utils_7.3+2+nmu1_i386.deb
Files: 
 f069b08c3d0a853e4b02a9536bb4ee9c 1432 x11 optional x11-utils_7.3+2+nmu1.dsc
 2d560d4182a0f70b92aae8f5bbe10bc5 2134376 x11 optional x11-utils_7.3+2+nmu1.tar.gz
 a410aed4f86463bcd58b5b3d5234ff6a 202558 x11 optional x11-utils_7.3+2+nmu1_i386.deb

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iD8DBQFJX+qc79ZNCRIGYgcRAsptAJ9nf2NgJPD+VoshdihKev1xQZWUngCeMYpX
PufXyRJ2YOpM+WX+bAJTnzM=
=ALB5
-----END PGP SIGNATURE-----

Message #39 received at 508126@bugs.debian.org (full text, mbox, reply):

From: Ben Hutchings <ben@decadent.org.uk>

To: Julien Cristau <jcristau@debian.org>

Cc: 508126@bugs.debian.org, debian-release@lists.debian.org

Subject: Re: Bug#508126: xine-ui: ctrl/shift key press emulation implementation broken

Date: Sun, 04 Jan 2009 01:34:21 +0000

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

On Sat, 2009-01-03 at 18:00 +0100, Julien Cristau wrote:
> On Fri, Jan  2, 2009 at 15:30:01 +0000, Ben Hutchings wrote:
> 
> > #508126: x11-utils: xprop -spy does not handle destruction properly
> > 
> the xprop patch looks reasonable afaict, so go ahead and NMU.  somebody
> should also forward that patch to bugs.freedesktop.org.

Thanks, done.  The NMU-diff follows.

Ben.

diff -Nru x11-utils-7.3+2/debian/changelog x11-utils-7.3+2+nmu1/debian/changelog
--- x11-utils-7.3+2/debian/changelog	2008-05-30 15:21:43.000000000 +0100
+++ x11-utils-7.3+2+nmu1/debian/changelog	2009-01-03 22:22:41.000000000 +0000
@@ -1,3 +1,11 @@
+x11-utils (7.3+2+nmu1) unstable; urgency=low
+
+  * Non-maintainer upload.
+  * Make xprop -spy exit cleanly when target window is destroyed
+    (closes: #508126)
+
+ -- Ben Hutchings <ben@decadent.org.uk>  Sat, 03 Jan 2009 22:22:40 +0000
+
 x11-utils (7.3+2) unstable; urgency=low
 
   * Relax Replaces on xutils and xbase-clients to allow further updates.
diff -Nru x11-utils-7.3+2/debian/patches/05_xprop_spy_exit_on_destroy.diff x11-utils-7.3+2+nmu1/debian/patches/05_xprop_spy_exit_on_destroy.diff
--- x11-utils-7.3+2/debian/patches/05_xprop_spy_exit_on_destroy.diff	1970-01-01 01:00:00.000000000 +0100
+++ x11-utils-7.3+2+nmu1/debian/patches/05_xprop_spy_exit_on_destroy.diff	2009-01-03 22:21:48.000000000 +0000
@@ -0,0 +1,50 @@
+This patch by Ben Hutchings <ben@decadent.org.uk>.
+
+xprop -spy should exit cleanly when the target window is destroyed.
+The current behaviour is that it sometimes dies with a BadWindow error
+and sometimes hangs around after the target has been destroyed.
+
+We fix this by listening for destroy events but also catching
+BadWindow errors (and BadMatch, which may sometimes be received
+instead of BadWindow).  We print a new-line before exiting from the
+error handler, since we may have generated partial output for a
+property change.
+
+--- x11-utils.orig/xprop/xprop.c
++++ x11-utils/xprop/xprop.c
+@@ -1596,6 +1596,19 @@
+ 
+ static int spy = 0;
+ 
++static int (*old_error_handler)(Display *dpy, XErrorEvent *ev);
++
++static int spy_error_handler(Display *dpy, XErrorEvent *ev)
++{
++    if (ev->error_code == BadWindow || ev->error_code == BadMatch) {
++	/* Window was destroyed */
++	puts("");
++	exit(0);
++    }
++
++    return old_error_handler(dpy, ev);
++}
++
+ int
+ main (int argc, char **argv)
+ {
+@@ -1738,9 +1750,14 @@
+ 	XEvent event;
+ 	const char *format, *dformat;
+ 	
+-	XSelectInput(dpy, target_win, PropertyChangeMask);
++	XSelectInput(dpy, target_win, PropertyChangeMask | StructureNotifyMask);
++	old_error_handler = XSetErrorHandler(spy_error_handler);
+ 	for (;;) {
+ 	    XNextEvent(dpy, &event);
++	    if (event.type == DestroyNotify)
++		break;
++	    if (event.type != PropertyNotify)
++		continue;
+ 	    format = dformat = NULL;
+ 	    if (props) {
+ 		int i;
diff -Nru x11-utils-7.3+2/debian/patches/series x11-utils-7.3+2+nmu1/debian/patches/series
--- x11-utils-7.3+2/debian/patches/series	2008-02-01 00:21:49.000000000 +0000
+++ x11-utils-7.3+2+nmu1/debian/patches/series	2009-01-03 22:22:05.000000000 +0000
@@ -1,2 +1,3 @@
 02_xev_flush_standard_output.diff
 04_xlsfonts_do_not_spew_usage_on_connection_error.diff -p0
+05_xprop_spy_exit_on_destroy.diff
--- END ---

-- 
Ben Hutchings
The world is coming to an end.  Please log off.

[signature.asc (application/pgp-signature, inline)]

Message #48 received at 508126@bugs.debian.org (full text, mbox, reply):

From: Luk Claes <luk@debian.org>

To: Ben Hutchings <ben@decadent.org.uk>

Cc: Julien Cristau <jcristau@debian.org>, 508126@bugs.debian.org, debian-release@lists.debian.org

Subject: Re: Bug#508126: xine-ui: ctrl/shift key press emulation implementation broken

Date: Sun, 04 Jan 2009 17:08:22 +0100

Ben Hutchings wrote:
> On Sat, 2009-01-03 at 18:00 +0100, Julien Cristau wrote:
>> On Fri, Jan  2, 2009 at 15:30:01 +0000, Ben Hutchings wrote:
>>
>>> #508126: x11-utils: xprop -spy does not handle destruction properly
>>>
>> the xprop patch looks reasonable afaict, so go ahead and NMU.  somebody
>> should also forward that patch to bugs.freedesktop.org.

unblocked

Cheers

Luk

Send a report that this bug log contains spam.