[PATCH v2 08/10] dri2: Send events only to known clients

[Pauli Nieminen]

On 14/02/11 21:06 +0200, Ville Syrjälä wrote:
> On Tue, Feb 08, 2011 at 11:42:54PM +0200, ext Pauli wrote:
> > From: Pauli Nieminen <ext-pauli.nieminen at nokia.com>
> > 
> > If client disconnects and new client gets same id DRI2 events may end to
> > wrong client. DRI2 reference list can be checked to see if the client
> > still owns the DRI2Drawable.
> > 
> > Signed-off-by: Pauli Nieminen <ext-pauli.nieminen at nokia.com>
> > ---
> >  hw/xfree86/dri2/dri2.c |   73 +++++++++++++++++++++++++++++++++++------------
> >  1 files changed, 54 insertions(+), 19 deletions(-)
> > 
> > diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
> > index 9133bb3..c77ac9b 100644
> > --- a/hw/xfree86/dri2/dri2.c
> > +++ b/hw/xfree86/dri2/dri2.c
> > @@ -111,12 +111,16 @@ typedef struct _DRI2Screen {
> >      ConfigNotifyProcPtr		 ConfigNotify;
> >  } DRI2ScreenRec;
> >  
> > +typedef struct _DRI2ClientEventRec {
> > +    ClientPtr		client;
> > +    struct list		link;
> > +} DRI2ClientEventRec, *DRI2ClientEventPtr;
> > +
> >  typedef struct _DRI2SwapCompleteDataRec {
> >      DRI2BufferPtr	src;
> >      DRI2BufferPtr	dest;
> > -    ClientPtr		client;
> >      void *		data;
> > -    struct list		link;
> > +    DRI2ClientEventRec	event;
> >  } DRI2SwapCompleteDataRec, *DRI2SwapCompleteDataPtr;
> >  
> >  static struct list *
> > @@ -266,7 +270,8 @@ DRI2LookupClientDrawableRef(DRI2DrawablePtr pPriv, ClientPtr client, XID id)
> >      DRI2DrawableRefPtr ref;
> >  
> >      list_for_each_entry(ref, &pPriv->reference_list, link) {
> > -	if (CLIENT_ID(ref->dri2_id) == client->index && ref->id == id)
> > +	if (CLIENT_ID(ref->dri2_id) == client->index &&
> > +		(id == 0 || ref->id == id))
> >  	    return ref;
> >      }
> >      return NULL;
> > @@ -753,28 +758,41 @@ DRI2CanExchange(DrawablePtr pDraw)
> >      return FALSE;
> >  }
> >  
> > +static void
> > +cleanup_client_event(DRI2ClientEventPtr event)
> > +{
> > +    if (event->client)
> > +	list_del(&event->link);
> > +}
> > +
> >  void
> >  DRI2WaitMSCComplete2(DRI2DrawablePtr pPriv, int frame,
> >  		     unsigned int tv_sec, unsigned int tv_usec,
> >  		     void *data)
> >  {
> > -    ClientPtr client = data;
> > +    ClientPtr          blockedClient = pPriv->blockedClient;
> > +    DRI2ClientEventPtr pEvent = data;
> > +    ClientPtr          client = pEvent->client;
> >  
> > +    pPriv->blockedClient = NULL;
> > +    pPriv->blockedOnMsc = FALSE;
> >      pPriv->refcnt--;
> >  
> > -    if (pPriv->refcnt <= 0) {
> > -	DRI2DrawableGone(pPriv, 0);
> > -	return;
> > -    }
> > +    if (client == NULL)
> > +	goto out;
> >  
> >      ProcDRI2WaitMSCReply(client, ((CARD64)tv_sec * 1000000) + tv_usec,
> >  			 frame, pPriv->swap_count);
> >  
> > -    if (pPriv->blockedClient)
> > -	AttendClient(pPriv->blockedClient);
> > +    if (blockedClient)
> > +	AttendClient(blockedClient);
> >  
> > -    pPriv->blockedClient = NULL;
> > -    pPriv->blockedOnMsc = FALSE;
> > +out:
> > +    cleanup_client_event(pEvent);
> > +    free(pEvent);
> > +
> > +    if (pPriv->refcnt <= 0)
> > +	DRI2DrawableGone(pPriv, 0);
> >  }
> >  
> >  static void
> > @@ -816,7 +834,7 @@ DRI2WakeClient(ClientPtr client, DRI2DrawablePtr pPriv, int frame,
> >  static void
> >  free_swap_complete_data (DRI2DrawablePtr pPriv, DRI2SwapCompleteDataPtr pSwapData)
> >  {
> > -    list_del(&pSwapData->link);
> > +    cleanup_client_event(&pSwapData->event);
> >      buffer_unref(pPriv, pSwapData->src);
> >      buffer_unref(pPriv, pSwapData->dest);
> >      free(pSwapData);
> > @@ -828,9 +846,10 @@ DRI2SwapComplete2(DRI2DrawablePtr pPriv, int frame,
> >  		   DRI2SwapEventPtr swap_complete, void *swap_data)
> >  {
> >      DRI2SwapCompleteDataPtr pSwapData = swap_data;
> > -    ClientPtr               client = pSwapData->client;
> > +    ClientPtr               client = pSwapData->event.client;
> >      DrawablePtr             pDraw = pPriv->drawable;
> >      CARD64                  ust = 0;
> > +    DRI2DrawableRefPtr      ref;
> >  
> >      pPriv->swapsPending--;
> >      pPriv->swap_count++;
> > @@ -841,6 +860,11 @@ DRI2SwapComplete2(DRI2DrawablePtr pPriv, int frame,
> >      if (client == NULL)
> >  	goto out;
> >  
> > +    ref = DRI2LookupClientDrawableRef(pPriv, client, 0);
> > +
> > +    if (ref == NULL)
> > +	goto out;
> 
> This bit escapes me. pSwapData->event.client is not NULL, so we know
> the original client is still around. Are we now assuming that
> DRI2SwapComplete2 was somehow called with an incorrect pSwapData
> since we have to go looking for the client's ID in the ref list?
> 

We still have to check that client haven't called DRIDestroyDrawable. In that
case client shouldn't receive "random" DRI2 events.

> > +
> >      if (pDraw) {
> >  	BoxRec          box;
> >  	RegionRec       region;
> > @@ -934,9 +958,9 @@ DRI2SwapBuffers(ClientPtr client, DRI2DrawablePtr pPriv, CARD64 target_msc,
> >      buffer_ref(pDestBuffer);
> >      pSwapData->src = pSrcBuffer;
> >      pSwapData->dest = pDestBuffer;
> > -    pSwapData->client = client;
> >      pSwapData->data = data;
> > -    list_add (&pSwapData->link, clientEvents);
> > +    pSwapData->event.client = client;
> > +    list_add (&pSwapData->event.link, clientEvents);
> >  
> >      /* Old DDX or no swap interval, just blit */
> >      if (!ds->ScheduleSwap || !pPriv->swap_interval) {
> > @@ -1062,13 +1086,22 @@ int
> >  DRI2WaitMSC(ClientPtr client, DRI2DrawablePtr pPriv, CARD64 target_msc,
> >  	    CARD64 divisor, CARD64 remainder)
> >  {
> > -    DRI2ScreenPtr ds = pPriv->dri2_screen;
> > -    DrawablePtr   pDraw = pPriv->drawable;
> > +    DRI2ScreenPtr      ds = pPriv->dri2_screen;
> > +    DrawablePtr        pDraw = pPriv->drawable;
> > +    struct list *      clientEvents = DRI2GetClientEventList(client);
> > +    DRI2ClientEventPtr pEvent;
> >      Bool ret;
> >  
> >      if (pDraw == NULL)
> >  	return BadDrawable;
> >  
> > +    pEvent = malloc(sizeof *pEvent);
> > +    if (!pEvent)
> > +	return BadAlloc;
> > +
> > +    pEvent->client = client;
> > +    list_add(&pEvent->link, clientEvents);
> > +
> >      pPriv->refcnt++;
> >  
> >      /* Old DDX just completes immediately */
> > @@ -1081,6 +1114,8 @@ DRI2WaitMSC(ClientPtr client, DRI2DrawablePtr pPriv, CARD64 target_msc,
> >      ret = (*ds->ScheduleWaitMSC)(client, pDraw, target_msc, divisor, remainder, client);
> >      if (!ret) {
> >  	pPriv->refcnt--;
> > +	cleanup_client_event(pEvent);
> > +	free(pEvent);
> >  	return BadDrawable;
> >      }
> >  
> > @@ -1316,7 +1351,7 @@ DRI2ClientCallback(CallbackListPtr *ClientStateCallback, pointer closure, pointe
> >      NewClientInfoRec *clientinfo = calldata;
> >      ClientPtr pClient = clientinfo->client;
> >      struct list *clientEvents = DRI2GetClientEventList(pClient);
> > -    DRI2SwapCompleteDataPtr ref, next;
> > +    DRI2ClientEventPtr ref, next;
> >  
> >      switch (pClient->clientState) {
> >  	case ClientStateInitial:
> > -- 
> > 1.7.0.4
> > 
> > _______________________________________________
> > xorg-devel at lists.x.org: X.Org development
> > Archives: http://lists.x.org/archives/xorg-devel
> > Info: http://lists.x.org/mailman/listinfo/xorg-devel
> 
> -- 
> Ville Syrjälä