[PATCH 1/2] OS support: fix writeable client vs IgnoreClient behavior

[Jesse Barnes]

On Mon, 28 Jun 2010 17:16:51 -0700
Keith Packard <keithp at keithp.com> wrote:

> On Mon, 28 Jun 2010 16:59:03 -0700, Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
> > On Mon, 28 Jun 2010 16:55:30 -0700
> > Keith Packard <keithp at keithp.com> wrote:
> > 
> > > On Mon, 28 Jun 2010 15:58:46 -0700, Jesse Barnes <jbarnes at virtuousgeek.org> wrote:
> > > 
> > > > When ResetCurrentRequest is called, or IgnoreClient is called when a
> > > > client has input pending, IgnoredClientsWithInput will be set.  However,
> > > > a subsequent IgnoreClient request will clear the client fd from that fd
> > > > set, potentially causing the client to hang.
> > > 
> > > Sounds like we need a counter for IgnoreClient that leaves the client
> > > ignored until it gets back to zero.
> > 
> > Yeah, that was another approach I coded up, it worked just as well.
> > Either way I suppose their could be code depending on the current
> > behavior, that's what I'm most worried about.
> 
> A counter would make nesting Ignore/Attend client calls work better,
> right? Your plan would wake the client at the first AttendClient call,
> rather than waiting for all of the suspending conditions to clear.

Well a simple counter wouldn't be enough to avoid the bug here.  We'd
need to avoid any changing of the masks if IgnoreClient had already
been called as well.

This is really a question of which behavior we want to preserve:
  1) clearing of IgnoredClientsWithInput across multiple IgnoreClient
  calls
or
  2) allowing multiple IgnoreClient and AttendClient calls to have any
  effect at all except when first ignoring and finally attending

Here's what the counter patch looks like.  It needs to be matched with
a change to my DRI2 patch to unconditionally call AttendClient so the
counts match up.

-- 
Jesse Barnes, Intel Open Source Technology Center

diff --git a/dix/dispatch.c b/dix/dispatch.c
index 982c808..70b1a86 100644
--- a/dix/dispatch.c
+++ b/dix/dispatch.c
@@ -3668,6 +3668,7 @@ void InitClient(ClientPtr client, int i, pointer ospriv)
     client->smart_start_tick = SmartScheduleTime;
     client->smart_stop_tick = SmartScheduleTime;
     client->smart_check_tick = SmartScheduleTime;
+    client->ignoreCount = 0;
 }
 
 /************************
diff --git a/include/dixstruct.h b/include/dixstruct.h
index 696b793..568ea1f 100644
--- a/include/dixstruct.h
+++ b/include/dixstruct.h
@@ -99,6 +99,7 @@ typedef struct _Client {
     int         clientGone;
     int         noClientException;	/* this client died or needs to be
 					 * killed */
+    int         ignoreCount;		/* count for Attend/IgnoreClient */
     SaveSetElt	*saveSet;
     int         numSaved;
     void	*unused_screenPrivate[16];
diff --git a/os/connection.c b/os/connection.c
index 61ba72a..dd48c11 100644
--- a/os/connection.c
+++ b/os/connection.c
@@ -1148,6 +1148,11 @@ IgnoreClient (ClientPtr client)
     int connection = oc->fd;
 
     isItTimeToYield = TRUE;
+
+    client->ignoreCount++;
+    if (client->ignoreCount > 1)
+	return;
+
     if (!GrabInProgress || FD_ISSET(connection, &AllClients))
     {
     	if (FD_ISSET (connection, &ClientsWithInput))
@@ -1181,6 +1186,11 @@ AttendClient (ClientPtr client)
 {
     OsCommPtr oc = (OsCommPtr)client->osPrivate;
     int connection = oc->fd;
+
+    client->ignoreCount--;
+    if (client->ignoreCount)
+	return;
+
     if (!GrabInProgress || GrabInProgress == client->index ||
 	FD_ISSET(connection, &GrabImperviousClients))
     {