GNOME Bugzilla – Bug 745289
wayland: do not use g_error() on connection errors
Last modified: 2017-05-30 07:17:29 UTC
Created attachment 298092 [details] [review] Patch to fix the issue. When the Wayland compositor vanishes, all applications connected will receive a SIGPIPE as soon as they try to use wl_display_dispatch(). Do not use g_error() to terminate the applications when this occurs, g_error() means an error in the application while here it's not truly the case: https://developer.gnome.org/glib/stable/glib-Message-Logging.html#g-error "[...] This function will result in a core dump; don't use it for errors you expect. Using this function indicates a bug in your program, i.e. an assertion failure." This can lead automated bug reporting tools to automatically log bugs for various components even though this is perfectly normal (at least not a bug in the components)... So instead of g_error(), use g_printerr() and exit() just like other backend do in such case (e.g. Broadway backend).
Review of attachment 298092 [details] [review]: ::: gdk/wayland/gdkeventsource.c @@ +162,3 @@ { + if (wl_display_dispatch (display_wayland->wl_display) < 0) { + g_printerr ("Error dispatching display: %s\n", g_strerror (errno)); As mentioned on irc: Probably better to say "Wayland display" here. @@ +168,3 @@ + if (source->pfd.revents & (G_IO_ERR | G_IO_HUP)) { + g_printerr ("Lost connection to wayland compositor\n"); And "Wayland compositor" here.
Created attachment 298098 [details] [review] Updated patch (In reply to Matthias Clasen from comment #1)
Review of attachment 298098 [details] [review]: Looks similar to: https://bugzilla.redhat.com/show_bug.cgi?id=1366897 https://bugzilla.gnome.org/show_bug.cgi?id=783047 (updating the patch's status to reflect reality) ::: gdk/wayland/gdkeventsource.c @@ +164,3 @@ + { + g_warning ("Error %d (%s) dispatching to Wayland display.", + errno, g_strerror (errno)); Maybe we should use g_message instead? See https://bugzilla.redhat.com/show_bug.cgi?id=1366897#c43 for a rationale. The X11 backend (gdk_x_io_error) also uses g_message these days. @@ +165,3 @@ + g_warning ("Error %d (%s) dispatching to Wayland display.", + errno, g_strerror (errno)); + exit (1); Also, maybe, _exit instead?
Created attachment 352565 [details] [review] wayland: Don't warn if the display is lost, skip exit handlers
Review of attachment 352565 [details] [review]: looks fine to me and in line with the other error case.
Comment on attachment 352565 [details] [review] wayland: Don't warn if the display is lost, skip exit handlers Pushed to master and gtk-3-22.
Thanks, Olivier!