[Cogl] [PATCH 1/2] wayland: Remove the Wayland socket FD if there are any errors

Neil Roberts neil at linux.intel.com
Tue Jul 9 11:26:30 PDT 2013


Previously if the Wayland socket gets closed then Cogl would ignore
the error when dispatching events which meant the socket would be
constantly ready for reading, the main loop would never go idle and it
would sit at 100% CPU. When Wayland encounters an error it will
actually close the socket which means if something else opened another
file then we might even end up polling on a completely unrelated FD.
This patch makes it remove the FD from the main loop as soon as it
hits an error so that it will at least avoid breaking the main loop.
However I think most applications would probably want to abort in this
case so we might also want to add a way to inform the application of
this or even just abort directly.

The cogl_poll_* functions have been changed so that they can cope if
the pending and dispatch callbacks remove their own FD while they are
invoked.
---
 cogl/cogl-poll.c                      | 24 +++++++++----
 cogl/winsys/cogl-winsys-egl-wayland.c | 65 +++++++++++++++++++++++++++--------
 2 files changed, 69 insertions(+), 20 deletions(-)

diff --git a/cogl/cogl-poll.c b/cogl/cogl-poll.c
index cb62327..f1c19f0 100644
--- a/cogl/cogl-poll.c
+++ b/cogl/cogl-poll.c
@@ -46,23 +46,26 @@ cogl_poll_renderer_get_info (CoglRenderer *renderer,
                              int *n_poll_fds,
                              int64_t *timeout)
 {
-  GList *l;
+  GList *l, *next;
 
   _COGL_RETURN_VAL_IF_FAIL (cogl_is_renderer (renderer), 0);
   _COGL_RETURN_VAL_IF_FAIL (poll_fds != NULL, 0);
   _COGL_RETURN_VAL_IF_FAIL (n_poll_fds != NULL, 0);
   _COGL_RETURN_VAL_IF_FAIL (timeout != NULL, 0);
 
-  *poll_fds = (void *)renderer->poll_fds->data;
-  *n_poll_fds = renderer->poll_fds->len;
   *timeout = -1;
 
   if (!_cogl_list_empty (&renderer->idle_closures))
     *timeout = 0;
 
-  for (l = renderer->poll_sources; l; l = l->next)
+  /* This loop needs to cope with the prepare callback removing its
+   * own fd */
+  for (l = renderer->poll_sources; l; l = next)
     {
       CoglPollSource *source = l->data;
+
+      next = l->next;
+
       if (source->prepare)
         {
           int64_t source_timeout = source->prepare (source->user_data);
@@ -72,6 +75,11 @@ cogl_poll_renderer_get_info (CoglRenderer *renderer,
         }
     }
 
+  /* This is deliberately set after calling the prepare callbacks in
+   * case one of them removes its fd */
+  *poll_fds = (void *)renderer->poll_fds->data;
+  *n_poll_fds = renderer->poll_fds->len;
+
   return renderer->poll_fds_age;
 }
 
@@ -80,17 +88,21 @@ cogl_poll_renderer_dispatch (CoglRenderer *renderer,
                              const CoglPollFD *poll_fds,
                              int n_poll_fds)
 {
-  GList *l;
+  GList *l, *next;
 
   _COGL_RETURN_IF_FAIL (cogl_is_renderer (renderer));
 
   _cogl_closure_list_invoke_no_args (&renderer->idle_closures);
 
-  for (l = renderer->poll_sources; l; l = l->next)
+  /* This loop needs to cope with the dispatch callback removing its
+   * own fd */
+  for (l = renderer->poll_sources; l; l = next)
     {
       CoglPollSource *source = l->data;
       int i;
 
+      next = l->next;
+
       if (source->fd == -1)
         {
           source->dispatch (source->user_data, 0);
diff --git a/cogl/winsys/cogl-winsys-egl-wayland.c b/cogl/winsys/cogl-winsys-egl-wayland.c
index f5a6047..075c258 100644
--- a/cogl/winsys/cogl-winsys-egl-wayland.c
+++ b/cogl/winsys/cogl-winsys-egl-wayland.c
@@ -132,13 +132,28 @@ prepare_wayland_display_events (void *user_data)
 
   flush_ret = wl_display_flush (wayland_renderer->wayland_display);
 
-  /* If the socket buffer became full then we need to wake up the main
-   * loop once it is writable again */
-  if (flush_ret == -1 && errno == EAGAIN)
-    _cogl_poll_renderer_modify_fd (renderer,
-                                   wayland_renderer->fd,
-                                   COGL_POLL_FD_EVENT_IN |
-                                   COGL_POLL_FD_EVENT_OUT);
+  if (flush_ret == -1)
+    {
+      /* If the socket buffer became full then we need to wake up the
+       * main loop once it is writable again */
+      if (errno == EAGAIN)
+        {
+          _cogl_poll_renderer_modify_fd (renderer,
+                                         wayland_renderer->fd,
+                                         COGL_POLL_FD_EVENT_IN |
+                                         COGL_POLL_FD_EVENT_OUT);
+        }
+      else if (errno != EINTR)
+        {
+          /* If the flush failed for some other reason then it's
+           * likely that it's going to consistently fail so we'll stop
+           * waiting on the file descriptor instead of making the
+           * application take up 100% CPU. FIXME: it would be nice if
+           * there was some way to report this to the application so
+           * that it can quit or recover */
+          _cogl_poll_renderer_remove_fd (renderer, wayland_renderer->fd);
+        }
+    }
 
   /* Calling this here is a bit dodgy because Cogl usually tries to
    * say that it won't do any event processing until
@@ -159,19 +174,41 @@ dispatch_wayland_display_events (void *user_data, int revents)
   CoglRendererWayland *wayland_renderer = egl_renderer->platform;
 
   if ((revents & COGL_POLL_FD_EVENT_IN))
-    wl_display_dispatch (wayland_renderer->wayland_display);
+    {
+      if (wl_display_dispatch (wayland_renderer->wayland_display) == -1 &&
+          errno != EAGAIN &&
+          errno != EINTR)
+        goto socket_error;
+    }
 
   if ((revents & COGL_POLL_FD_EVENT_OUT))
     {
       int ret = wl_display_flush (wayland_renderer->wayland_display);
 
-      if (ret != -1 || errno != EAGAIN)
-        /* There is no more data to write so we don't need to wake up
-         * when the write buffer is emptied anymore */
-        _cogl_poll_renderer_modify_fd (renderer,
-                                       wayland_renderer->fd,
-                                       COGL_POLL_FD_EVENT_IN);
+      if (ret == -1)
+        {
+          if (errno != EAGAIN && errno != EINTR)
+            goto socket_error;
+        }
+      else
+        {
+          /* There is no more data to write so we don't need to wake
+           * up when the write buffer is emptied anymore */
+          _cogl_poll_renderer_modify_fd (renderer,
+                                         wayland_renderer->fd,
+                                         COGL_POLL_FD_EVENT_IN);
+        }
     }
+
+  return;
+
+ socket_error:
+  /* If there was an error on the wayland socket then it's likely that
+   * it's going to consistently fail so we'll stop waiting on the file
+   * descriptor instead of making the application take up 100% CPU.
+   * FIXME: it would be nice if there was some way to report this to
+   * the application so that it can quit or recover */
+  _cogl_poll_renderer_remove_fd (renderer, wayland_renderer->fd);
 }
 
 static CoglBool
-- 
1.7.11.3.g3c3efa5



More information about the Cogl mailing list