On 2018-05-08 09:47 PM, Adam Jackson wrote:
> dri*_bind_context, when switching current drawables, will drop the
> reference on the old one; since that refcount has probably now gone to
> zero that means we lose all the state we applied to that drawable
> before, like when swaps are expected to complete.
> 
> Dropping this reference might make some sense for drawables that aren't
> _ours_, since we don't get events for destroyed resources and need to
> rely on the server throwing errors when we name a no-longer-valid
> drawable. But if the resource is one that this client created, we can be
> reasonably sure that it will be explicitly destroyed by the same client

Is there any mechanism in place for this to result in
loader_dri3_drawable_fini actually getting called when destroying a
window without using glXDestroyWindow?


> - and if not, the client is likely to exit anyway, so the memory leak
> doesn't matter.>
> So, bump the refcnt if the XID of the drawable indicates that it's one
> of ours. This is, admittedly, a hack. The proper solution would involve
> rather more surgery to the MakeCurrent path than I can type quickly, let
> alone test promptly against a wide enough range of servers and DRIs to
> have any confidence in. I'll work on the real solution, but in the
> meantime this is effectively not a memory leak for any real scenario,
> and fixes a real bug.

An application which opens and closes many windows doesn't seem that
unrealistic to me, and I suspect many (most?) GLX users still don't use
glXDestroyWindow, so I'm not sure I can agree with this assessment.


> @@ -449,6 +450,8 @@ driFetchDrawable(struct glx_context *gc, GLXDrawable 
> glxDrawable)
>        return NULL;
>     }
>     pdraw->refcount = 1;
> +   if ((glxDrawable & dpy->resource_mask) == dpy->resource_base)

Should this be

   if ((glxDrawable & ~dpy->resource_mask) == dpy->resource_base)

(Negated dpy->resource_mask)? Otherwise this fails to take effect for me
here with glxgears:

(gdb) p/x glxDrawable
$1 = 0x200002
(gdb) p/x dpy->resource_mask
$2 = 0x1fffff
(gdb) p/x dpy->resource_base
$3 = 0x200000


With the code changed as above, pdraw->refcount is incremented to 2, and
then never drops to 0 with glxgears, and loader_dri3_drawable_fini is
never called, although glxgears explicitly calls XDestroyWindow.


How about the idea I described before, saving the loader_dri3_drawable
values in a hash table in dri3_screen? Maybe we could simply save a
pointer to the whole struct, after only freeing the renderbuffers.


-- 
Earthling Michel Dänzer               |               http://www.amd.com
Libre software enthusiast             |             Mesa and X developer
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to