GNOME Bugzilla – Bug 622236
_shell_app_remove_window() may crash the shell
Last modified: 2010-06-21 13:45:47 UTC
If _shell_app_remove_window() is called for an app which is not running the shell segfaults. While it is weird that an app which is not running can have open windows, it is possible (at least for now). The underlying problem is that an app is considered running if at least one window has been added to it by shell_window_tracker - which is done only for windows considered "interesting" (no transients, skip-taskbar, etc). The easiest solution is to apply the same logic when removing a window, but alternative approaches might be clearer (e.g. the filtering for "interesting" windows could be moved into shell-app - uninteresting windows can be interesting, see e.g. bug 602669) First patch reverts an arguably wrong commit, the second contains the fix outlined above.
Created attachment 164170 [details] [review] Revert "[ShellApp] Move assertion below precondition check" The precondition check segfaults if the assertion would fail. It's either plague or cholera, but an assert seems a tiny bit nicer than a segfault. This reverts commit 1f550dbc72a771ebe682714e9c8d6777b2af9fda.
Created attachment 164171 [details] [review] [windowTracker] Only remove "interesting" windows Windows are only added to an application if they are considered "interesting". If we keep it that way, we cannot unconditionally call _shell_app_remove_window() - applications without interesting windows are not considered running, so the call crashes the shell.
Review of attachment 164171 [details] [review]: I'd prefer the two patches get squashed, otherwise we have an intermediate broken state again. Otherwise looks fine. In the future I think it'd be cleanest if the app held a list of all windows, did the interesting filtering internally.
Squashed & pushed Attachment 164171 [details] pushed as 4b1fea2 - [windowTracker] Only remove "interesting" windows
(In reply to comment #3) > In the future I think it'd be cleanest if the app held a list of all windows, > did the interesting filtering internally. Agreed.