[RFC] glx: fix DRI2 memory leak

[Magnus Kessler]

On Wednesday 25 March 2009, Jesse Barnes wrote:
> In trying to track down the memory leak in 20704, I found that the DRI2
> drawable destroy routine doesn't seem to get called when new drawables
> are created and old ones destroyed (as in the glViewport case in the
> bug).
>
> The GLX core code takes care of destroying drawables correctly though,
> and even calls DestroyPixmap at the right time.  However, DRI2
> drawables have more state associated with them than just a single
> pixmap, so we have a __glXDRIdrawableDestroy function that takes care
> of that.  However, by the time we get there, the GLX drawable is gone,
> so I never saw the
>     if (drawable->pDraw != NULL)
> 	DRI2DestroyDrawable(drawable->pDraw);
> case get triggered...
>
> The simple patch below fixed that, but apparently isn't correct (see
> the bug report) since it causes crashes after a time.  My patch was
> missing another change to set the private->count correctly in
> dri2GetBuffers though, which may be part of the fix as well.
>
> Has anyone else seen this leak?  Anyone care to educate me a bit more
> about GLX drawable lifetime rules?
>
> Thanks,
> Jesse

Hi Jesse,

This memory leak has troubled me for some time, as it forces me to log out 
of my X sessions on a regular basis. I have tested your patch and it seems 
to help tremendously. However, digging through the git commits for glxext.c 
it appears that the line setting glxPriv->pDraw to NULL was added as part of 
commit 
http://cgit.freedesktop.org/xorg/xserver/commit/?id=30bcaa966d6b00f1630609a78db18dee683cc43d 
which aims to "Make glx destroy path handle cases where the X window goes 
away first". So it might be that the commenter on bug 20704 who observes a 
crash hits this condition. Maybe Kristian Høgsberg can comment some more.

Thanks,

Magnus


>
> diff --git a/glx/glxext.c b/glx/glxext.c
> index c882372..73e5a9b 100644
> --- a/glx/glxext.c
> +++ b/glx/glxext.c
> @@ -127,9 +127,9 @@ static Bool DrawableGone(__GLXdrawable *glxPriv, XID
> xid) break;
>      }
>
> -    glxPriv->pDraw = NULL;
>      glxPriv->drawId = 0;
>      __glXUnrefDrawable(glxPriv);
> +    glxPriv->pDraw = NULL;
>
>      return True;
>  }
> _______________________________________________
> xorg mailing list
> xorg at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/xorg


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.x.org/archives/xorg/attachments/20090326/5b45fcd2/attachment.pgp>