On Tue, Jul 09, 2013 at 06:58:32PM +0100, Neil Roberts wrote: > Bill Spitzak <[email protected]> writes: > > > Maybe it can abort if ...read_events() is called after an error was > > encountered. Then bad toolkits will exit, while ones that check for the > > error state can do something intelligent. > > This sounds like a nice idea but it might go a bit wrong if there are > orthogonal bits of code reading the events. For example if the error is > first detected in eglSwapBuffers() then the first time the application > will know about it is when it later calls wl_display_dispatch, but that > will already be the second thing that tries to read the error so it > would abort. > > That actually highlights a more serious problem. As soon as the error is > encountered libwayland-client will actually close the socket. That means > that eglSwapBuffers might cause the socket to close without the > application having a chance to know about it. It will then try to poll > on an invalid socket. Or worse still, something might try to open a file > in the interim and end up reusing the file descriptor. Then it would > actually poll on a random unrelated file and completely miss the error!
Argh yes, it's bad to close the fd in display_fatal_error(), we need to keep it open until the application destroys the wl_display. I was thinking that maybe that's also why we're not catching POLLHUP from poll, but it turns out it's much sillier than that: commit 99b78fdd7f21539a288bfe846542b633756ce163 Author: Kristian Høgsberg <[email protected]> Date: Tue Jul 9 18:35:06 2013 -0400 wayland: Don't clear revents until we've checked for G_IO_HUP https://bugzilla.gnome.org/show_bug.cgi?id=703892 diff --git a/gdk/wayland/gdkeventsource.c b/gdk/wayland/gdkeventsource.c index cb335bd..4bcae9b 100644 --- a/gdk/wayland/gdkeventsource.c +++ b/gdk/wayland/gdkeventsource.c @@ -150,12 +150,12 @@ _gdk_wayland_display_queue_events (GdkDisplay *display) display_wayland = GDK_WAYLAND_DISPLAY (display); source = (GdkWaylandEventSource *) display_wayland->event_source; + if (source->pfd.revents & G_IO_IN) - { - wl_display_dispatch(display_wayland->wl_display); - source->pfd.revents = 0; - } + wl_display_dispatch (display_wayland->wl_display); if (source->pfd.revents & (G_IO_ERR | G_IO_HUP)) g_error ("Lost connection to wayland compositor"); + + source->pfd.revents = 0; } but your patches (wayland and gtk+) still make sense of course. I pushed your wayland patch with Robs wording tweak and my follow-up fix to not return -1 when we get EAGAIN, and I pushed both your gtk+ fix and the above gtk+ fix. And both gtk+ fixes are necessary - we can get G_IO_IN | G_IO_HUP, in which case wl_display_dispatch() will read whatever data we got up until the HUP and return 0 and the G_IO_HUP test will the catch the error. Or we may get another error from wl_display_dispatch() that isn't reflected in the revents flags. Kristian > Egads, > - Neil > --------------------------------------------------------------------- > Intel Corporation (UK) Limited > Registered No. 1134945 (England) > Registered Office: Pipers Way, Swindon SN3 1RJ > VAT No: 860 2173 47 > > This e-mail and any attachments may contain confidential material for > the sole use of the intended recipient(s). Any review or distribution > by others is strictly prohibited. If you are not the intended > recipient, please contact the sender and delete all copies. > > _______________________________________________ > wayland-devel mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/wayland-devel _______________________________________________ wayland-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/wayland-devel
