[Mesa-dev] [PATCH 6/7] glx: remove incorrect refcounting of DRIdrawable
Ian Romanick
idr at freedesktop.org
Tue Feb 9 16:02:59 UTC 2016
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 <marcheu at chromium.org>
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 <piman at chromium.org>
Signed-off-by: Stéphane Marchesin <marcheu at chromium.org>
Reviewed-by: Adam Jackson <ajax at redhat.com>
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. :(
>> this fixes fd.o bug #93955 and disappearing characters in KotOR when
>> soft shadows are enabled
>
> I think this is also a candidate for stable. Reference this in the
> commit message like:
>
> This fixes disappearing characters in KotOR when soft shadows are enabled.
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=93955
> Cc: "11.1" <mesa-stable at lists.freedesktop.org>
>
>> ---
>> src/glx/dri2_glx.c | 4 ----
>> src/glx/dri3_glx.c | 4 ----
>> src/glx/dri_common.c | 38 --------------------------------------
>> src/glx/dri_glx.c | 4 ----
>> src/glx/drisw_glx.c | 4 ----
>> src/glx/glxclient.h | 1 -
>> 6 files changed, 55 deletions(-)
>>
>> diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
>> index 7710349..ebc878f 100644
>> --- a/src/glx/dri2_glx.c
>> +++ b/src/glx/dri2_glx.c
>> @@ -122,8 +122,6 @@ dri2_destroy_context(struct glx_context *context)
>> struct dri2_context *pcp = (struct dri2_context *) context;
>> struct dri2_screen *psc = (struct dri2_screen *) context->psc;
>>
>> - driReleaseDrawables(&pcp->base);
>> -
>> free((char *) context->extensions);
>>
>> (*psc->core->destroyContext) (pcp->driContext);
>> @@ -145,8 +143,6 @@ dri2_bind_context(struct glx_context *context, struct glx_context *old,
>> pdraw = (struct dri2_drawable *) driFetchDrawable(context, draw);
>> pread = (struct dri2_drawable *) driFetchDrawable(context, read);
>>
>> - driReleaseDrawables(&pcp->base);
>> -
>> if (pdraw)
>> dri_draw = pdraw->driDrawable;
>> else if (draw != None)
>> diff --git a/src/glx/dri3_glx.c b/src/glx/dri3_glx.c
>> index 6054ffc..38f5799 100644
>> --- a/src/glx/dri3_glx.c
>> +++ b/src/glx/dri3_glx.c
>> @@ -186,8 +186,6 @@ dri3_destroy_context(struct glx_context *context)
>> struct dri3_context *pcp = (struct dri3_context *) context;
>> struct dri3_screen *psc = (struct dri3_screen *) context->psc;
>>
>> - driReleaseDrawables(&pcp->base);
>> -
>> free((char *) context->extensions);
>>
>> (*psc->core->destroyContext) (pcp->driContext);
>> @@ -206,8 +204,6 @@ dri3_bind_context(struct glx_context *context, struct glx_context *old,
>> pdraw = (struct dri3_drawable *) driFetchDrawable(context, draw);
>> pread = (struct dri3_drawable *) driFetchDrawable(context, read);
>>
>> - driReleaseDrawables(&pcp->base);
>> -
>> if (pdraw == NULL || pread == NULL)
>> return GLXBadDrawable;
>>
>> diff --git a/src/glx/dri_common.c b/src/glx/dri_common.c
>> index 6728d38..2d334ab 100644
>> --- a/src/glx/dri_common.c
>> +++ b/src/glx/dri_common.c
>> @@ -404,7 +404,6 @@ driFetchDrawable(struct glx_context *gc, GLXDrawable glxDrawable)
>> return NULL;
>>
>> if (__glxHashLookup(priv->drawHash, glxDrawable, (void *) &pdraw) == 0) {
>> - pdraw->refcount ++;
>> return pdraw;
>> }
>>
>> @@ -420,47 +419,10 @@ driFetchDrawable(struct glx_context *gc, GLXDrawable glxDrawable)
>> (*pdraw->destroyDrawable) (pdraw);
>> return NULL;
>> }
>> - pdraw->refcount = 1;
>>
>> return pdraw;
>> }
>>
>> -_X_HIDDEN void
>> -driReleaseDrawables(struct glx_context *gc)
>> -{
>> - const struct glx_display *priv = gc->psc->display;
>> - __GLXDRIdrawable *pdraw;
>> -
>> - if (priv == NULL)
>> - return;
>> -
>> - if (__glxHashLookup(priv->drawHash,
>> - gc->currentDrawable, (void *) &pdraw) == 0) {
>> - if (pdraw->drawable == pdraw->xDrawable) {
>> - pdraw->refcount --;
>> - if (pdraw->refcount == 0) {
>> - (*pdraw->destroyDrawable)(pdraw);
>> - __glxHashDelete(priv->drawHash, gc->currentDrawable);
>> - }
>> - }
>> - }
>> -
>> - if (__glxHashLookup(priv->drawHash,
>> - gc->currentReadable, (void *) &pdraw) == 0) {
>> - if (pdraw->drawable == pdraw->xDrawable) {
>> - pdraw->refcount --;
>> - if (pdraw->refcount == 0) {
>> - (*pdraw->destroyDrawable)(pdraw);
>> - __glxHashDelete(priv->drawHash, gc->currentReadable);
>> - }
>> - }
>> - }
>> -
>> - gc->currentDrawable = None;
>> - gc->currentReadable = None;
>> -
>> -}
>> -
>> _X_HIDDEN bool
>> dri2_convert_glx_attribs(unsigned num_attribs, const uint32_t *attribs,
>> unsigned *major_ver, unsigned *minor_ver,
>> diff --git a/src/glx/dri_glx.c b/src/glx/dri_glx.c
>> index d087751..69d7502 100644
>> --- a/src/glx/dri_glx.c
>> +++ b/src/glx/dri_glx.c
>> @@ -526,8 +526,6 @@ dri_destroy_context(struct glx_context * context)
>> struct dri_context *pcp = (struct dri_context *) context;
>> struct dri_screen *psc = (struct dri_screen *) context->psc;
>>
>> - driReleaseDrawables(&pcp->base);
>> -
>> free((char *) context->extensions);
>>
>> (*psc->core->destroyContext) (pcp->driContext);
>> @@ -547,8 +545,6 @@ dri_bind_context(struct glx_context *context, struct glx_context *old,
>> pdraw = (struct dri_drawable *) driFetchDrawable(context, draw);
>> pread = (struct dri_drawable *) driFetchDrawable(context, read);
>>
>> - driReleaseDrawables(&pcp->base);
>> -
>> if (pdraw == NULL || pread == NULL)
>> return GLXBadDrawable;
>>
>> diff --git a/src/glx/drisw_glx.c b/src/glx/drisw_glx.c
>> index 241ac7f..2350292 100644
>> --- a/src/glx/drisw_glx.c
>> +++ b/src/glx/drisw_glx.c
>> @@ -234,8 +234,6 @@ drisw_destroy_context(struct glx_context *context)
>> struct drisw_context *pcp = (struct drisw_context *) context;
>> struct drisw_screen *psc = (struct drisw_screen *) context->psc;
>>
>> - driReleaseDrawables(&pcp->base);
>> -
>> free((char *) context->extensions);
>>
>> (*psc->core->destroyContext) (pcp->driContext);
>> @@ -254,8 +252,6 @@ drisw_bind_context(struct glx_context *context, struct glx_context *old,
>> pdraw = (struct drisw_drawable *) driFetchDrawable(context, draw);
>> pread = (struct drisw_drawable *) driFetchDrawable(context, read);
>>
>> - driReleaseDrawables(&pcp->base);
>> -
>> if (pdraw == NULL || pread == NULL)
>> return GLXBadDrawable;
>>
>> diff --git a/src/glx/glxclient.h b/src/glx/glxclient.h
>> index 2776b44..9105be3 100644
>> --- a/src/glx/glxclient.h
>> +++ b/src/glx/glxclient.h
>> @@ -134,7 +134,6 @@ struct __GLXDRIdrawableRec
>> GLenum textureTarget;
>> GLenum textureFormat; /* EXT_texture_from_pixmap support */
>> unsigned long eventMask;
>> - int refcount;
>> };
>>
>> /*
>>
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
More information about the mesa-dev
mailing list