GNOME Bugzilla – Bug 648132
Workspace[Thumbnail]._doAddWindow() can be called after workspace is gone
Last modified: 2011-04-25 20:46:00 UTC
Looking at the backtrace in: https://bugzilla.redhat.com/show_bug.cgi?id=658684 Which shows an idle calling to meta_workspace_index() which asserts because the workspace has been removed, what I think can happen is that Workspace[Thumbnail]._doAddWindow() sometimes add an idle to add a window if it isn't yet managed by the compositor, but that idle can fire after the workspace (and the window perhaps too) are already gone. Both the "workspace gone" and "window gone" part can, I think, be handled by a check: window.get_workspace() == this.metaWorkspace Since: - When a workspace is removed, all windows are removed from it - a workspace can't be removed with windows still on it. - When a window is unmanaged, it's workspace is set to NULL. (See the check in meta_window_unmanage() that g_asserts() that window->workspace == NULL) That check will also fix the case where the window is rapidly moved between workspaces at creation time even if the workspace isn't removed. [ Separately, - I think the check in Mutter probably should be a g_return_if_fail() not a meta_bug() - Every case where we use an idle_add() in Mutter is a bug, since idle_add() isn't called deterministically. ]
Created attachment 186390 [details] [review] workspace: avoid an assertion in a new window/deleted workspace race If a window appeared on a workspace that was then quickly removed, we could hit an assertion in mutter. Avoid that. ==== Owen's reasoning seems right. There's one other possibility though: if the window is on_all_workspaces, then metaWin.get_workspace() will return the current workspace. But that should also be ok, since presumably the current workspace can't be one that is destroyed. I've tested that this doesn't break anything, but I don't really know that it fixes the downstream crash.
Correct link to downstream bug is: https://bugzilla.redhat.com/show_bug.cgi?id=697575 The link above is wrong.
Created attachment 186450 [details] Test case for reproducing bug Run this program, go to the overview, and hit change work spaces key options (c-a-up/down) while near the last workspace which is being added/removed and you'll quickly trigger a crash with this assertion. Seems to be fixed with the patch I'll attach which is your patch but also fixes the corresponding code in workspaceThumbnail.
Created attachment 186451 [details] [review] workspace: avoid an assertion in a new window/deleted workspace race If a window appeared on a workspace that was then quickly removed, we could hit an assertion in mutter. Avoid that.
Comment on attachment 186451 [details] [review] workspace: avoid an assertion in a new window/deleted workspace race ok
Pushed my version with the workspaceThumbnail additions Attachment 186451 [details] pushed as e38d83f - workspace: avoid an assertion in a new window/deleted workspace race