On Fre, 2010-12-10 at 12:43 +0000, Chris Wilson wrote: > Although there may be more than one resource handles pointing to the > Drawable, we only want to destroy it once and only reference the > resource which may have just been deleted on the first instance. > > Signed-off-by: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> > Cc: Kristian HÃgsberg <krh@xxxxxxxxxxxxx> > Cc: Michel DÃnzer <daenzer@xxxxxxxxxx> > --- > glx/glxcmds.c | 6 ++++++ > glx/glxdrawable.h | 3 +++ > glx/glxext.c | 15 ++++++++++----- > 3 files changed, 19 insertions(+), 5 deletions(-) > > diff --git a/glx/glxcmds.c b/glx/glxcmds.c > index de9c3f0..fa153d5 100644 > --- a/glx/glxcmds.c > +++ b/glx/glxcmds.c > @@ -1139,6 +1143,8 @@ DoCreateGLXDrawable(ClientPtr client, __GLXscreen *pGlxScreen, > pGlxDraw->destroy (pGlxDraw); > return BadAlloc; > } > + pGlxDraw->refcnt++; > + pGlxDraw->otherId = pDraw->id; I think this should use drawableId instead of pDraw->id, and refcnt should only be incremented if it's different from glxDrawableId. Combined with the patch I attached to https://bugs.freedesktop.org/show_bug.cgi?id=28181 this block could look something like if (drawableId != glxDrawableId) { /* Add the glx drawable under the XID of the underlying X drawable * too. That way we'll get a callback in DrawableGone and can * clean up properly when the drawable is destroyed. */ if (!AddResource(drawableId, __glXDrawableRes, pGlxDraw)) { pGlxDraw->destroy (pGlxDraw); return BadAlloc; } pGlxDraw->refcnt++; pGlxDraw->otherId = pDraw->id; } Then this patch will hopefully address that bug and maybe others with references to DrawableGone. Fingers crossed. :) > diff --git a/glx/glxext.c b/glx/glxext.c > index 4bd5d6b..c473aa9 100644 > --- a/glx/glxext.c > +++ b/glx/glxext.c > @@ -128,13 +128,18 @@ static Bool DrawableGone(__GLXdrawable *glxPriv, XID xid) > * constructors, we added it as a glx drawable resource under both > * its glx drawable ID and it X drawable ID. Remove the other > * resource now so we don't a callback for freed memory. */ > - if (glxPriv->drawId != glxPriv->pDraw->id) { > - if (xid == glxPriv->drawId) > - FreeResourceByType(glxPriv->pDraw->id, __glXDrawableRes, TRUE); > - else > - FreeResourceByType(glxPriv->drawId, __glXDrawableRes, TRUE); > + if (glxPriv->otherId) { > + XID other = glxPriv->otherId; > + glxPriv->otherId = 0; > + if (xid == other) > + FreeResourceByType(glxPriv->drawId, __glXDrawableRes, TRUE); > + else > + FreeResourceByType(other, __glXDrawableRes, TRUE); > } > > + if (--glxPriv->refcnt) > + return TRUE; The return at the end of the function uses True instead of TRUE. -- Earthling Michel DÃnzer | http://www.vmware.com Libre software enthusiast | Debian, X and DRI developer _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/dri-devel
