[RFC] glx: fix DRI2 memory leak

[Shuang He]

Magnus Kessler wrote:
> On Monday 30 March 2009, Shuang He wrote:
>   
>> 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
>>     
>
> I have observed this crash once as well, but haven't yet found a way to 
> reproduce it. Can you post a few steps that make this crash more likely? On 
> the bug report (http://bugs.freedesktop.org/show_bug.cgi?id=20704) you 
> mention it has something to do with resizing by small amounts.
>
> Thanks
>
> Magnus
>   
I could easily reproduce this by just resizing the window several 
times.  Most of times, it's just costing 1 or 2 minutes.

Thanks
    --Shuang
>
> _______________________________________________
> xorg mailing list
> xorg at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/xorg
>   

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