GNOME Bugzilla – Bug 108108
update_sm_hints loop detection logic doesn't work
Last modified: 2004-12-22 21:47:04 UTC
There is a feeble attempt to catch loops in update_sm_hints(): while (w != NULL) { leader = read_client_leader (window->display, w->xwindow); if (leader != None) break; if (w->xtransient_for == None || w->transient_parent_is_root_window) break; w = meta_display_lookup_x_window (w->display, w->xtransient_for); if (w == window) break; /* Cute, someone thought they'd make a transient_for cycle */ } But this obviously only catches the case where the leader points it itself, it doesn't catch the A=>B=>B case, which is what has metacity stuck in a server grab and restuck in a server grab when killed and respawned... In fact, you also need to catch the A=>B=>C=>B case, so you should use the typical tortoise-and-hare cycle detection algorithm... that is soemthing along the lines of: w = y = window; while (TRUE) { w = leader (w); if (!w || w == y) break; w = leader (w); if (!w || w == y) break; y = leader (y); } op_start is the client that caused it, though I'm not sure the exact sequence that got things set up in this fashion.
Frederic, is this your bug 107806?
I was just running metacity in gdb and I was about to post my finding about it.. This seems to be the problem I'm seeing on Mdk systems..
Cool, glad that's tracked down. I wonder why it doesn't happen to me (why my version of Qt doesn't seem to do this). If Qt is setting dialogs transient for themselves, someone should really file a Qt bug as well. Though no question metacity should be robust against it.
We are using QT 3.1.1 for Mandrake 9.1 According to its changelog (http://www.trolltech.com/developer/changes/changes-3.1.2.html) , QT 3.1.2 sets WM_CLIENT_LEADER and SM_CLIENT_ID properties according to the ICCCM (section 5.1) Unfortunalely, this version has some issues with other parts of KDE, so we couldn't ship it with Mandrake 9.1 :(( And I'm still not sure it fixes this problem (our KDE hacker tested it kickly and he was still able to cause the infinite loop in metacity).. For re-reading Owen analysis, I almost 100% sure this bug is the freeze people are seeing : ksnapshot causes the loop when it popups "Save as" dialog, which probably set ksnapshot main window as client leader and since this main window has the client leader wrongly set, bingo.. If you ever need me to test a patch, don't hesitate to ping me :)
Since this loop detection needs to be in several places here is my first draft at factoring it out, if someone wants to double-check it. I'm working on a patch based on this. void meta_window_foreach_ancestor (MetaWindow *window, MetaWindowForeachFunc func, void *data) { MetaWindow *w; MetaWindow *tortoise; w = window; tortoise = window; while (TRUE) { if (w->xtransient_for == None || w->transient_parent_is_root_window) break; w = meta_display_lookup_x_window (w->display, w->xtransient_for); if (w == NULL || w == tortoise) break; if (!(* func) (w, data)) break; if (w->xtransient_for == None || w->transient_parent_is_root_window) break; w = meta_display_lookup_x_window (w->display, w->xtransient_for); if (w == NULL || w == tortoise) break; if (!(* func) (w, data)) break; tortoise = meta_display_lookup_x_window (tortoise->display, tortoise->xtransient_for); /* "w" should have already covered all ground covered by the * tortoise, so the following must hold. */ g_assert (tortoise != NULL); g_assert (tortoise->xtransient_for != None); g_assert (!tortoise->transient_parent_is_root_window); } }
*** Bug 106217 has been marked as a duplicate of this bug. ***
Created attachment 14954 [details] [review] Proposed fix
Yeppe !! It works perfectly.. Many thanks to both Owen and Havoc..
The logic in meta_window_foreach_ancestor() looks right to me.
*** Bug 108326 has been marked as a duplicate of this bug. ***
Applied