After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 593644 - gdk_x11_screen_get_window_manager_name should not cache the result
gdk_x11_screen_get_window_manager_name should not cache the result
Status: RESOLVED FIXED
Product: gtk+
Classification: Platform
Component: Backend: X11
2.17.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2009-08-31 10:08 UTC by Travis Watkins
Modified: 2009-09-05 00:26 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Reduce wm name cache timeout (490 bytes, patch)
2009-09-01 12:39 UTC, Christian Dywan
none Details | Review

Description Travis Watkins 2009-08-31 10:08:52 UTC
In gdkevents-x11.c gdk_x11_screen_get_window_manager_name calls fetch_net_wm_check_window which if successful sets screen_x11->need_refetch_wm_name to true which is then used in the first function to decide if it should check for the WM name again or use the previously looked up value.

The problem with this setup is that fetch_net_check_window first checks to see if 15 seconds have passed since the last time it was called. In Ubuntu's gnome-appearance-properties we check what WM is running, start compiz, wait 8 seconds, then check again to see if compiz started. Since 8 seconds is less than 15 seconds we always get "Metacity" back as the current WM name.

My current thought on this is that there doesn't seem to be any reason to cache this information as it is quick to get and not used often.
Comment 1 Christian Dywan 2009-09-01 12:39:54 UTC
Created attachment 142240 [details] [review]
Reduce wm name cache timeout

The question is, are there applications which rely on the caching and call the function very often for some reason? Maybe reducing the timeout is a compromise.

Otherwise, what about the window-manager-changed signal? Wouldn't that be more reliable in any case?
Comment 2 Travis Watkins 2009-09-01 16:31:21 UTC
A smaller timeout is fine but I suppose the real problem is either that the cache exists in the first place or that it is not properly getting invalidated when the current WM goes away. There does seem to be some code to handle that but the 15 second check must override it.
Comment 3 Matthias Clasen 2009-09-03 02:49:50 UTC
First of all, as Christian says, you should just listen for GdkScreen:window-manager-changed. Even so, I don't see how you are still getting the name of the dead wm after 8 seconds, considering this code:

  if (screen_x11 && screen_x11->wmspec_check_window != None &&
      xwindow == screen_x11->wmspec_check_window)
    {
      if (xevent->type == DestroyNotify)
        {
          screen_x11->wmspec_check_window = None;
          g_free (screen_x11->window_manager_name);
          screen_x11->window_manager_name = g_strdup ("unknown");

          /* careful, reentrancy */
          _gdk_x11_screen_window_manager_changed (GDK_SCREEN (screen_x11));
        }

Are you saying that you are not getting the DestroyNotify for metacitys wmspec_check_window ?


What this code probably should do is reset last_wmspec_check_time to 0, to ensure that the next fetch_net_wm_check_window call _does_ check - after all, in this case we know that the window has changed...

Can you try if that fixes your problem ?
Comment 4 Travis Watkins 2009-09-04 16:23:30 UTC
Hmm, strange, with or without that change my test app seems to be working now. However in our patch to gnome-control-center we switched to making the calls to Xlib directly and that fixed the problem there. Not sure what is going on at this point...
Comment 5 Matthias Clasen 2009-09-05 00:26:15 UTC
commit 5c14089225e5706af90e0c195d2e95886bef6418
Author: Matthias Clasen <mclasen@redhat.com>
Date:   Fri Sep 4 20:22:43 2009 -0400

    Make window manager tracking work better
    
    We were getting the new wm name with a 15 second delay, due to
    some race. Reported in bug 593644.