[RFC] glx: fix DRI2 memory leak

[Shuang He]

Jesse Barnes wrote:
> On Fri, 27 Mar 2009 13:20:25 -0400
> Kristian Høgsberg <krh at bitplanet.net> wrote:
>
>   
>> On Thu, Mar 26, 2009 at 6:33 PM, Jesse Barnes
>> <jbarnes at virtuousgeek.org> wrote:
>>     
>>> Ok, I think this is where the leak was:
>>> __glXUnrefDrawable->DrawableGone->__glXUnrefDrawable.  This sequence
>>> meant the glxPriv would stay around (since it was used), but
>>> DrawableGone had generally already disposed of the pDrawable before
>>> the call to Unref, so we never got into DRI2DestroyBuffers, thus
>>> the leak.
>>>
>>> The loop seems broken to me.  It *looks* like DrawableGone should be
>>> the real free routine, only called when a drawable really is free,
>>> so I've fixed up the code to conform to that.  This means removing
>>> the GLX priv frees from DRI and DRI2 routines and putting them here
>>> and using the GLX unref routine everwhere (since it will only free
>>> the resource when the refcount reaches 0).
>>>
>>> Any thoughts?  I'd appreciate some more testers too...  I'm not
>>> quite sure about the generic DoDestroyDrawable change, but if the
>>> refcount is always 1 there anyway it shouldn't make a difference
>>> and seems more correct.
>>>       
>> The __GLXdrawable is a reference counted object, and the glx code
>> references it in a couple of places: when there's an X resource
>> pointing to it (a GLXWindow, GLXPixmap or GLXPbuffer) and when it's
>> the current drawable or readable for a context.  The __GLXdrawable is
>> allocated by the backend in use (dri, dri2 or swrast) and must be
>> freed by the same backend (don't mix alloc and free across abstraction
>> barriers).  We unref the __GLXdrawable when we either set a different
>> drawable/readable and when the X resource goes away.
>>
>> The leak is (as you pointed out before) because we NULL the pDraw
>> pointer before calling the backend destructor, which then can't unref
>> the DRI2Drawable, which we then leak.  You had the right fix in the
>> originial post, we just need to not touch glxPriv after
>> __glXUnrefDrawable if it got destroyed.  I suggest this change to
>> DrawableGone in irc:
>>
>>     refcount = glxPriv->refcount;
>>     __glXUnrefDrawable(glxPriv);
>>     if (refcount > 1) {
>>         glxPriv->pDraw = NULL;
>>         glxPriv->drawId = 0;
>>     }
>>     
>
> Yep, that seems to work too...  Magnus or Vasily can you guys confirm?
>
> Thanks,
>   
Memory leak problem seems resolved, but I still see X crash with:

(gdb) bt
#0  0xb7fd5424 in __kernel_vsyscall ()
#1  0x03155660 in raise () from /lib/libc.so.6
#2  0x03157028 in abort () from /lib/libc.so.6
#3  0x031925bd in __libc_message () from /lib/libc.so.6
#4  0x031987e4 in malloc_printerr () from /lib/libc.so.6
#5  0x0319c441 in _int_realloc () from /lib/libc.so.6
#6  0x0319d176 in realloc () from /lib/libc.so.6
#7  0x08131002 in Xrealloc (ptr=0x6, amount=0) at utils.c:1133
#8  0x0806d10b in dixAllocatePrivate (privates=0x91487c8, key=0xb7e90a3c)
    at privates.c:129
#9  0x0806d1cc in dixSetPrivate (privates=0x91487c8, key=0xb7e90a3c, val=0x0)
    at privates.c:193
#10 0xb7e8eca1 in DRI2DestroyDrawable (pDraw=0x91487b0) at dri2.c:218
#11 0xb7eee668 in __glXDRIdrawableDestroy (drawable=0x9205ff0) at glxdri2.c:108
#12 0xb7ee49bb in __glXUnrefDrawable (glxPriv=0x9205ff0) at glxutil.c:58
#13 0xb7ee3183 in DrawableGone (glxPriv=0x9205ff0, xid=12583326)
    at glxext.c:131
#14 0x0806efdc in FreeResource (id=12583326, skipDeleteFuncType=0)
    at resource.c:561
#15 0xb7edffa6 in DoDestroyDrawable (cl=<value optimized out>,
    glxdrawable=12583326, type=1) at glxcmds.c:1225
#16 0xb7ee340a in __glXDispatch (client=0x8d79db8) at glxext.c:523
#17 0x080874cf in Dispatch () at dispatch.c:437
---Type <return> to continue, or q <return> to quit---
#18 0x0806c69d in main (argc=2, argv=0xbf9d2754, envp=Cannot access memory at
address 0xbde

Thanks
	--Shuang
) at main.c:397


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.x.org/archives/xorg/attachments/20090330/40f1717f/attachment.html>