[PATCH 1/2] os/connection: Call ClientReady() based on a level trigger rather than an edge trigger

[Keith Packard]

Jeremy Huddleston Sequoia <jeremyhu at apple.com> writes:

> On encountering an EAGAIN, FlushClient() re-adds the client to the
> pending list and returns, but FlushClient() doesn't get called again.
> We would expect it to be called via FlushAllOutput() via
> WaitForSomething(), but that only happens when NewOutputPending is
> set.  FlushAllOutput() also early-exits if NewOutputPending is not
> set.
>
> The only place that is setting NewOutputPending is ClientReady().  The
> problem there is that ClientReady() is called based on an edge trigger
> and not a level trigger.

Hrm. I think this is a problem with our emulation of edge triggers as we
don't tell the ospoll layer when we've detected that the client is not
writable.

This should fix that; it makes the edge re-trigger whenever we start
listening again:

diff --git a/os/ospoll.c b/os/ospoll.c
index b00d422..8c99501 100644
--- a/os/ospoll.c
+++ b/os/ospoll.c
@@ -352,10 +352,14 @@ ospoll_listen(struct ospoll *ospoll, int fd, int xevents)
         epoll_mod(ospoll, osfd);
 #endif
 #if POLL
-        if (xevents & X_NOTIFY_READ)
+        if (xevents & X_NOTIFY_READ) {
             ospoll->fds[pos].events |= POLLIN;
-        if (xevents & X_NOTIFY_WRITE)
+            ospoll->fds[pos].revents &= ~POLLIN;
+        }
+        if (xevents & X_NOTIFY_WRITE) {
             ospoll->fds[pos].events |= POLLOUT;
+            ospoll->fds[pos].revents &= ~POLLOUT;
+        }
 #endif
     }
 }

I did find a bug with the NewOutputPending flag. We don't actually flush
the client's buffer if there are still requests queued. This is a change
From the previous behavior.

diff --git a/os/io.c b/os/io.c
index ff0ec3d..c6db028 100644
--- a/os/io.c
+++ b/os/io.c
@@ -615,6 +615,8 @@ FlushAllOutput(void)
         if (!client_is_ready(client)) {
             oc = (OsCommPtr) client->osPrivate;
             (void) FlushClient(client, oc, (char *) NULL, 0);
+        } else {
+            NewOutputPending = TRUE;
         }
     }
 }

-- 
-keith
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 800 bytes
Desc: not available
URL: <https://lists.x.org/archives/xorg-devel/attachments/20160918/17df8095/attachment.sig>