Learn how easy it is to sync an existing GitHub or Google Code repo to a SourceForge project! See Demo

Close

#85 Rox pinpoard crashes with gtk2.0>=2.16.2

Crash
closed-works-for-me
nobody
ROX-Session (4)
5
2009-07-01
2009-06-25
M.A.G.I.E
No

Hi,

The call of gdk_x11_atom_to_xatom_for_display with GDK_NONE is very nasty as GDK_NONE is something that is returned by gdk in some special cases and should not be sent back to gtk (kick me if I'm wrong). Anyhow, due to some bug in gtk it used to be harmless to do so ( http://bugzilla.gnome.org/show_bug.cgi?id=580511 ) but as it has been corrected gdk_x11_atom_to_xatom_for_display instead of returning a valid xatom with "None" as id. returns anything but an xatom then XSendEvent() causes the server to kill the client.
The patch I propose will break this behaviour as near as possible of the calls of gdk_x11_atom_to_xatom_for_display() in rox_wmspec_change_state() (gui-support.c) and is quite dirty.
Some better thing to do would be to avoid calling gdk_x11_atom_to_xatom_for_display() with GDK_NONE as a parameter as it is done below in keep_below(). In that function prbably only state2 is likely to be equal to GDK_NONE as the call is explicitally done with this value, but I state both state1 and state2 in the patch I propose. Another way would be to test the validity of xatom before calling XSendEvent() that will not suppress gtk critical warning but would prevent application crashes.
Anyhow, my patch prevents both gtk critical warning (that experiment other gtk2 programs as iceape/seamonkey and bluefish for example) prevents the crash, and does not seem to modify the expected behaviour

Discussion

  • M.A.G.I.E
    M.A.G.I.E
    2009-06-25

    • labels: --> ROX-Session
    • milestone: --> Crash
     
  • M.A.G.I.E
    M.A.G.I.E
    2009-06-25

    Hi I forgot to sign. Please send mail to yl AT icite dot fr (or marcopolo-ter at users.sourceforge.net)

    Yves Lambert

     
  • M.A.G.I.E
    M.A.G.I.E
    2009-06-25

    « Some better thing to do would be to avoid calling
    gdk_x11_atom_to_xatom_for_display() with GDK_NONE as a parameter » I meant
    Some better thing to do would be to avoid calling
    *rox_wmspec_change_state()* with GDK_NONE as a parameter

     
  • Thomas Leonard
    Thomas Leonard
    2009-06-27

    The spec (http://standards.freedesktop.org/wm-spec/latest/ar01s05.html#id2569140) says:

    To change the state of a mapped window, a Client MUST send a _NET_WM_STATE client message to the root window:

    window = the respective client window
    message_type = _NET_WM_STATE
    format = 32
    data.l[0] = the action, as listed below
    data.l[1] = first property to alter
    data.l[2] = second property to alter
    data.l[3] = source indication
    other data.l[] elements = 0

    This message allows two properties to be changed simultaneously, specifically to allow both horizontal and vertical maximization to be altered together. l[2] MUST be set to zero if only one property is to be changed.

    So, I think only state2 should ever be GDK_NONE and we should use zero in this case.

    But, gdk_x11_atom_to_xatom_for_display() returns None in this case, which is #define'd to 0, on my system at least, so I don't understand why that would cause a problem.

    What number does it actually return for you?

     
  • M.A.G.I.E
    M.A.G.I.E
    2009-06-27

    Hii tal197, I did not report correctly the way to reproduce the crash:

    - gtk+ version must be 2.16.2 (doesn't occure with 2.16.1)
    Xorg version on which the crash occur are:
    debian : 1.6.1
    NetBSD 1.4.2
    rox version that crash:
    debian 2.8-1, git latest, NetBSD 2.4.1

    WM tested: fvwm-crystal, twm and w9wm

    'rox -S' or 'rox --pinboard=PIN' reproduce the crash with any of the wm I tested

    I tested git Head on debian (that is the version I dirtily patched) (I didn't find install.sh script in git repository) :
    using a dummy program that test what return gdk_x11_atom_to_xatom_for_display()
    and gdk_x11_atom_to_xatom:
    (gtk_taz:18213): Gdk-CRITICAL **: gdk_x11_atom_to_xatom_for_display: assertion `atom != GDK_NONE' failed
    gdk_x11_atom_to_xatom_for_display(display, GDK_NONE) returns a null value

    (gtk_taz:18213): Gdk-CRITICAL **: gdk_x11_atom_to_xatom_for_display: assertion `atom != GDK_NONE' failed
    gdk_x11_atom_to_xatom(GDK_NONE) returns a null value

    As seen in http://bugzilla.gnome.org/show_bug.cgi?id=580511 prior version of gdk_x11_atom_to_xatom_for_display returned an X atom with "NONE" identifier instead of None (0). This bug is fixed in 2.16.2.

    The guilty /might/ be XSendEvent(). My work around is very dirty as it reproduce almost the wrong behavior of previous versions of gdk_x11_atom_to_xatom_for_display() (but it prevents also GDKCritical warning) which lead a wrong event to be sent but unfortunately it looks like sending the standard event causes a crash. I wonder...

     
  • M.A.G.I.E
    M.A.G.I.E
    2009-06-27

    Should just prevent GDK CRITICAL warning but seems to do the job ???

     
    Attachments
  • M.A.G.I.E
    M.A.G.I.E
    2009-07-01

    My apologies:

    version prior to 2.8 (including debian's version) are crashing with GTK+ 2.16.2
    2.9 don't.
    I was mistaking because I am the worst programer since the epoch
    .
    The call to gdk_x11_atom_to_xatom_for_display prior to the crash is not in
    rox_wmspec_change_state() but in run_soap()

    break in runsoap()
    (gdb) next
    1295 retval = call->func(args);
    (gdb) nexti
    0x0809b881 1295 retval = call->func(args);
    (gdb) nexti
    0x0809b884 1295 retval = call->func(args);
    (gdb) nexti

    (ROX-Filer:24117): Gdk-CRITICAL **: gdk_x11_atom_to_xatom_for_display: assertion `atom != GDK_NONE' failed
    The program 'ROX-Filer' received an X Window System error.
    This probably reflects a bug in the program.
    The error was 'BadAtom (invalid Atom parameter)'.
    (Details: serial 974 error_code 5 request_code 20 minor_code 0)
    (Note to programmers: normally, X errors are reported asynchronously;
    that is, you will receive the error a while after causing it.
    To debug your program, run it with the --sync command line
    option to change this behavior. You can then get a meaningful
    backtrace from your debugger if you break on the gdk_x_error() function.)

    /* breaking on gdk_x_error() doesn't help)

    I dunno the value of call->func() (I am relly the worst) when it crashes anyhow, 2.9 doesn't crash anymore (as prior to 2.8 doesn't crash with gtk prior to 2.6.1)

     
  • M.A.G.I.E
    M.A.G.I.E
    2009-07-01

    • status: open --> closed-works-for-me