GNOME Bugzilla – Bug 751845
crash while initializing xwayland
Last modified: 2015-12-15 14:50:44 UTC
Created attachment 306638 [details] [review] Bind wayland socket after xwayland is initialized Mutter dispatches wayland events while starting Xwayland in order to let Xwayland connect to display and initialize itself. If some wayland client connects during this phase and creates surface, mutter crashes, because it is not still fully initialized. Attached patch moves binding of wayland socket after xwayland initialization, so that no client see it until xwayland is initialized and so no client can connect to mutter until it is initialized. Downstream bug: https://bugzilla.redhat.com/show_bug.cgi?id=1193489
Review of attachment 306638 [details] [review]: The change looks plausible to me, I'd like confirmation from someone more expert in Xwayland. A style note though. ::: src/wayland/meta-window-wayland.c @@ +342,3 @@ MetaWindow *window; + g_assert (display && "display is not initialized yet"); It's not clear why this needs a check more than any other place which takes a MetaDisplay - if you want to add an assertion, I'd add it xdg_shell_get_xdg_surface(), but honestly, when you hit this race condition, you had a pretty obvious crash, and I don't see the value of an explicit assert. @@ +343,3 @@ + g_assert (display && "display is not initialized yet"); + g_assert (surface && "no surface passed for new window"); And definitely no assert here. there's a pattern in GNOME for public APIs for these types of checks g_return_val_if_fail(META_IS_DISPLAY(display), NULL); g_return_val_if_fail(surface != NULL, NULL); to avoid an application developer corrupting GTK+ internals and having to debug into GTK+ for a simple mistake. But we don't do it for private Mutter APIs like this in most cases.
Review of attachment 306638 [details] [review]: Yeah. This patch looks good, but I don't want the asserts.
Created attachment 307194 [details] [review] Bind wayland socket after xwayland is initialized
Review of attachment 307194 [details] [review]: Looks good to commit ... sorry for missing the followup earlier.
Created attachment 316773 [details] [review] Bind wayland socket after xwayland is initialized rebased to master