Attachment 34603 Details for Bug 27406 – handle drawable destruction better

[patch] handle drawable destruction better

xserver-dri2-drawable-destruction-fixes.patch (text/plain), 4.07 KB, created by Jesse Barnes on 2010-04-01 15:39:39 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jesse Barnes

Created: 2010-04-01 15:39:39 UTC

Size: 4.07 KB

patch

obsolete

>commit 8476d99231cb725c090305d60f1c1c889d25c8dc
>Author: Jesse Barnes <jbarnes@virtuousgeek.org>
>Date:   Fri Mar 5 09:15:24 2010 -0800
>
>    DRI2: drawable lifetime fixes
>    
>    Handle drawable destruction and lifetime correctly.
>    
>    Check whether the drawable priv is valid in DRI2SwapInterval(),
>    DRI2WaitSBC() and DRI2WaitMSC(); it may have gone away, so be sure to
>    check it before using it.
>    
>    If more than 1 outstanding swap is queued, we may complete several after
>    an app has exited.  If we free it after the first one completes and the
>    refcount reaches 0, we'll crash the server on subsequent completions.
>    So delay freeing until all swaps complete and remove the error message
>    as this is a normal occurence.  To do this properly, we must also avoid
>    destroying drawables in DRI2DestroyDrawable() if a swap or wait event is
>    pending.
>    
>    And finally, make sure we free drawables in DRI2WaitMSCComplete() if
>    necessary (i.e. if the refcount has reached 0 and this MSC was the last
>    pending event on the object).
>    
>    Reported-by: Mario Kleiner <mario.kleiner@tuebingen.mpg.de>
>    Reviewed-by: Mario Kleiner <mario.kleiner@tuebingen.mpg.de>
>    Signed-off-by: Jesse Barnes <jbarnes@virtuousgeek.org>
>
>diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
>index 354b724..58f478f 100644
>--- a/hw/xfree86/dri2/dri2.c
>+++ b/hw/xfree86/dri2/dri2.c
>@@ -492,6 +492,10 @@ DRI2WaitMSCComplete(ClientPtr client, DrawablePtr pDraw, int frame,
> 	AttendClient(pPriv->blockedClient);
> 
>     pPriv->blockedClient = NULL;
>+
>+    /* If there's still a swap pending, let DRI2SwapComplete free it */
>+    if (pPriv->refCount == 0 && pPriv->swapsPending == 0)
>+	DRI2FreeDrawable(pDraw);
> }
> 
> static void
>@@ -543,13 +547,6 @@ DRI2SwapComplete(ClientPtr client, DrawablePtr pDraw, int frame,
> 	return;
>     }
> 
>-    if (pPriv->refCount == 0) {
>-        xf86DrvMsg(pScreen->myNum, X_ERROR,
>-		   "[DRI2] %s: bad drawable refcount\n", __func__);
>-	DRI2FreeDrawable(pDraw);
>-	return;
>-    }
>-
>     pPriv->swapsPending--;
>     pPriv->swap_count++;
> 
>@@ -561,6 +558,13 @@ DRI2SwapComplete(ClientPtr client, DrawablePtr pDraw, int frame,
>     pPriv->last_swap_ust = ust;
> 
>     DRI2WakeClient(client, pDraw, frame, tv_sec, tv_usec);
>+
>+    /*
>+     * It's normal for the app to have exited with a swap outstanding, but
>+     * don't free the drawable until they're all complete.
>+     */
>+    if (pPriv->swapsPending == 0 && pPriv->refCount == 0)
>+	DRI2FreeDrawable(pDraw);
> }
> 
> Bool
>@@ -669,10 +673,16 @@ DRI2SwapBuffers(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc,
> void
> DRI2SwapInterval(DrawablePtr pDrawable, int interval)
> {
>+    ScreenPtr       pScreen = pDrawable->pScreen;
>     DRI2DrawablePtr pPriv = DRI2GetDrawable(pDrawable);
> 
>-    /* fixme: check against arbitrary max? */
>+    if (pPriv == NULL) {
>+        xf86DrvMsg(pScreen->myNum, X_ERROR,
>+		   "[DRI2] %s: bad drawable\n", __func__);
>+	return;
>+    }
> 
>+    /* fixme: check against arbitrary max? */
>     pPriv->swap_interval = interval;
> }
> 
>@@ -721,7 +731,7 @@ DRI2WaitMSC(ClientPtr client, DrawablePtr pDraw, CARD64 target_msc,
>     Bool ret;
> 
>     pPriv = DRI2GetDrawable(pDraw);
>-    if (pPriv == NULL)
>+    if (pPriv == NULL || pPriv->refCount == 0)
> 	return BadDrawable;
> 
>     /* Old DDX just completes immediately */
>@@ -745,7 +755,7 @@ DRI2WaitSBC(ClientPtr client, DrawablePtr pDraw, CARD64 target_sbc,
>     DRI2DrawablePtr pPriv;
> 
>     pPriv = DRI2GetDrawable(pDraw);
>-    if (pPriv == NULL)
>+    if (pPriv == NULL || pPriv->refCount == 0)
> 	return BadDrawable;
> 
>     /* target_sbc == 0 means to block until all pending swaps are
>@@ -794,10 +804,10 @@ DRI2DestroyDrawable(DrawablePtr pDraw)
> 	xfree(pPriv->buffers);
>     }
> 
>-    /* If the window is destroyed while we have a swap pending, don't
>+    /* If the window is destroyed while we have a swap or wait pending, don't
>      * actually free the priv yet.  We'll need it in the DRI2SwapComplete()
>      * callback and we'll free it there once we're done. */
>-    if (!pPriv->swapsPending)
>+    if (!pPriv->swapsPending && !pPriv->blockedClient)
> 	DRI2FreeDrawable(pDraw);
> }
>

Actions: View | Splinter Review

Attachments on bug 27406: 34603

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.