After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 769578 - meta_xwayland_start() does late errno checks
meta_xwayland_start() does late errno checks
Status: RESOLVED FIXED
Product: mutter
Classification: Core
Component: wayland
git master
Other Linux
: Normal normal
: ---
Assigned To: mutter-maint
mutter-maint
Depends on:
Blocks:
 
 
Reported: 2016-08-06 14:41 UTC by Carlos Garnacho
Modified: 2016-08-06 15:18 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
xwayland: Avoid late errno checks (3.15 KB, patch)
2016-08-06 14:41 UTC, Carlos Garnacho
committed Details | Review

Description Carlos Garnacho 2016-08-06 14:41:09 UTC
Today gnome-shell --wayland --display-server was refusing to start here if started on a tty. It turned out to be the code in meta-xwayland.c that binds the sockets before starting Xwayland. X0 was already picked, so it'd correctly print:

(gnome-shell:23853): mutter-WARNING **: failed to bind to @/tmp/.X11-unix/X0: Address already in use

However it wouldn't loop to try and get another display's socket, I just got:

(gnome-shell:13780): mutter-ERROR **: Failed to start X Wayland

What turned out to happen is that errno is EADDRINUSE first, however it shifted into ENOTSOCK when we happen to perform the "should it continue" check. Some more investigation revealed that it's the g_warning() call itself which is changing errno, we also happen to do some other operations between the failing syscall we're reporting about and the use of errno.

So we should probably do the check for EADDRINUSE soon, and avoid relying on errno at later stages, I'm attaching a patch for that.
Comment 1 Carlos Garnacho 2016-08-06 14:41:43 UTC
Created attachment 332828 [details] [review]
xwayland: Avoid late errno checks

We do some things when binding to a socket fails (closing the fd,
logging, unlinking files, ...) those might affect errno in some
or other way, so it might no longer be EADDRINUSE even if we later
try to make those non fatal.

It seems better to check errno soon after the failure, and don't
rely on it in any way at a later point. All error paths in
bind_to_abstract_socket() also have early logging, which also might
help figure out better the point of failure when the socket fails
to be created.
Comment 2 Florian Müllner 2016-08-06 14:48:13 UTC
Review of attachment 332828 [details] [review]:

LGTM
Comment 3 Carlos Garnacho 2016-08-06 15:18:13 UTC
That was fast :). Fwiw glib changes errno when checking if stderr is a journald socket, so "not a bug" there I guess.

Attachment 332828 [details] pushed as 5c9a2c5 - xwayland: Avoid late errno checks