Control: forwarded -1
https://gitlab.gnome.org/GNOME/gnome-shell/merge_requests/497
On Sun, 07 Apr 2019 at 20:00:23 +0200, Bernhard Übelacker wrote:
> PS.: My untested change in message 10 might not crash, but lead to an
> infinitive loop, as app->running_state might not change anymore...
Yeah, let's not do that.
If line 1485 of shell_app_dispose() is indeed what's crashing,
then that would mean that app->running_state is non-NULL but
app->running_state->windows is NULL. This doesn't seem to be meant to
happen: we can see that in window_backed_app_get_window(), there's an
assertion that if app->running_state is non-NULL, then
app->running_state->windows is meant to be non-NULL too.
A common feature of upstream bugs 750 and 822 is that
_shell_app_remove_window() line 1110 is in the stack trace: this means
the ShellApp is being disposed while shell_app_sync_running_state() is
still running. At this point in _shell_app_remove_window(), window has
already been removed from app->running_state->windows, but
app->running_state has not yet been cleared: so the invariant that
app->running_state is only non-NULL if app->running_state->windows is also
non-NULL does not hold, and indeed in both those upstream bugs we seem
to have a violation of that invariant, causing a crash when user code
disposes the ShellApp in response to one of its signals.
I think the solution is probably to stop believing that
app->running_state != NULL implies app->running_state->windows != NULL,
and check for the latter whenever we need it; but the refcounting of the
ShellApp still seems suspicious, so I'm hoping for input from upstream
before uploading anything for this.
smcv
