On 12.02.2016 05:58, Miklós Máté wrote: > On 02/09/2016 05:02 PM, Ian Romanick wrote: >> On 02/08/2016 05:11 PM, Ian Romanick wrote: >>> On 02/05/2016 01:11 PM, Miklós Máté wrote: >>>> dri drawables must never be released when unbound from a context >>>> as long as their corresponding glx objects (window, pixmap, pbuffer) >>>> still exist >>> I'd really like to have Kristian weigh in, since DRI2 was his design, >>> and this is all his code being affected. That said, I'm a little unsure >>> about this change. >>> >>> The DRI drawables and associated resources will still get freed when the >>> application eventually calls glXDestroyPBuffer or glXDestroyWindow. >>> >>> But I'm not 100% sure what will happen with GLX 1.2 applications... >>> there are no glXDestroy* functions in GLX 1.2. In that case the >>> application will have a soft leak because calling XDestroyWindow won't >>> do anything on the client (or server?) for the DRI resources. I suspect >>> that's what this code was trying to prevent. >> And this commit affirms that: >> >> commit bf69ce37f0dcbb479078ee676d5100ac63e20750 >> Author: Stéphane Marchesin <[email protected]> >> Date: Wed Jun 15 15:09:12 2011 -0700 >> >> glx: implement drawable refcounting. >> The current dri context unbind logic will leak drawables >> until the process >> dies (they will then get released by the GEM code). There are two >> ways to fix >> this: either always call driReleaseDrawables every time we unbind >> a context >> (but that costs us round trips to the X server at getbuffers() >> time) or >> implement proper drawable refcounting. This patch implements the >> latter. >> Signed-off-by: Antoine Labour <[email protected]> >> Signed-off-by: Stéphane Marchesin <[email protected]> >> Reviewed-by: Adam Jackson <[email protected]> >> >> Since we don't have any way to know when the corresponding GLX object >> ceases to exist, we don't know when it's safe to destroy the DRI >> object. I don't know how we can not leak and not prematurely destroy >> the object. :( > That commit message exaggerates a little when it says "proper > refcounting", because it only refcounts on bind (in driFetchDrawable), > but not on create/destroy. With current Mesa the following happens: > create glXPbuffer: dri drawable is created, refcount=0 > bind it: refcount++ > unbind it: refcount--, drawable is destroyed > bind again: create new drawable, refcount=1 > unbind again: refcount--, drawable is destroyed > etc.
Could that be fixed by starting with refcount=1 instead of 0? > They also say that "current unbind logic will leak drawables until the > process dies", which is simply not true. The glXDestroy* calls properly > dispose of the corresponding dri objects. > > With GLX 1.3 the dri drawables are correctly created and destroyed along > with their corresponding glx objects even without the above commit. IMHO > if someone deliberately uses GLX 1.2 or earlier, they know what they are > doing (1.3 was released in 1998). I'd say that it's never safe to > destroy a dri object until it's explicitly destroyed by the user, so I'm > still convinced that my patch is correct. Unless I'm missing something, one issue I see with your patch is that a DRI drawable could be destroyed while it's still current for a context. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ mesa-dev mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/mesa-dev
