[PATCH] DRI2: handle drawable destruction properly at DRI2SwapComplete time

[Simon Thum]

Jesse Barnes wrote:
> Simon reported an issue with kwin that turned out to be a general problem.  If
> a drawable goes away before its swap completes, we'll try to free it up.
> However, we free it improperly, which causes a server crash in
> DRI2DestroyDrawable.  Fix that up by splitting the free code out and calling
> it from DRI2SwapComplete.
> 
> Tested-by: Simon Thum <simon.thum at gmx.de>
Note I didn't test it, maybe you're missing out someone? But feel free
to add my
  Reviewed-by: Simon Thum <simon.thum at gmx.de>
if there's no concurrency problem with
  xfree(pPriv);
being executed before
  dixSetPrivate(&pWin->devPrivates, dri2WindowPrivateKey, NULL);

(In case the swap stuff is parallelized somehow)


> Signed-off-by: Jesse Barnes <jbarnes at virtuousgeek.org>
> 
> diff --git a/hw/xfree86/dri2/dri2.c b/hw/xfree86/dri2/dri2.c
> index 3db826e..97c79d6 100644
> --- a/hw/xfree86/dri2/dri2.c
> +++ b/hw/xfree86/dri2/dri2.c
> @@ -157,6 +157,31 @@ DRI2CreateDrawable(DrawablePtr pDraw)
>      return Success;
>  }
>  
> +static void
> +DRI2FreeDrawable(DrawablePtr pDraw)
> +{
> +    DRI2DrawablePtr pPriv;
> +    WindowPtr  	    pWin;
> +    PixmapPtr	    pPixmap;
> +
> +    pPriv = DRI2GetDrawable(pDraw);
> +    if (pPriv == NULL)
> +	return;
> +
> +    xfree(pPriv);
> +
> +    if (pDraw->type == DRAWABLE_WINDOW)
> +    {
> +	pWin = (WindowPtr) pDraw;
> +	dixSetPrivate(&pWin->devPrivates, dri2WindowPrivateKey, NULL);
> +    }
> +    else
> +    {
> +	pPixmap = (PixmapPtr) pDraw;
> +	dixSetPrivate(&pPixmap->devPrivates, dri2PixmapPrivateKey, NULL);
> +    }
> +}
> +
>  static int
>  find_attachment(DRI2DrawablePtr pPriv, unsigned attachment)
>  {
> @@ -507,7 +532,7 @@ DRI2SwapComplete(ClientPtr client, DrawablePtr pDraw, int frame,
>      if (pPriv->refCount == 0) {
>          xf86DrvMsg(pScreen->myNum, X_ERROR,
>  		   "[DRI2] %s: bad drawable refcount\n", __func__);
> -	xfree(pPriv);
> +	DRI2FreeDrawable(pDraw);
>  	return;
>      }
>  
> @@ -728,8 +753,6 @@ DRI2DestroyDrawable(DrawablePtr pDraw)
>  {
>      DRI2ScreenPtr   ds = DRI2GetScreen(pDraw->pScreen);
>      DRI2DrawablePtr pPriv;
> -    WindowPtr  	    pWin;
> -    PixmapPtr	    pPixmap;
>  
>      pPriv = DRI2GetDrawable(pDraw);
>      if (pPriv == NULL)
> @@ -752,18 +775,7 @@ DRI2DestroyDrawable(DrawablePtr pDraw)
>       * 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)
> -	xfree(pPriv);
> -
> -    if (pDraw->type == DRAWABLE_WINDOW)
> -    {
> -	pWin = (WindowPtr) pDraw;
> -	dixSetPrivate(&pWin->devPrivates, dri2WindowPrivateKey, NULL);
> -    }
> -    else
> -    {
> -	pPixmap = (PixmapPtr) pDraw;
> -	dixSetPrivate(&pPixmap->devPrivates, dri2PixmapPrivateKey, NULL);
> -    }
> +	DRI2FreeDrawable(pDraw);
>  }
>  
>  Bool
> _______________________________________________
> xorg-devel mailing list
> xorg-devel at lists.x.org
> http://lists.x.org/mailman/listinfo/xorg-devel
>