| Message ID | 1292518458-3978-1-git-send-email-krh@bitplanet.net |
|---|---|
| State | Deferred, archived |
| Headers | show |
diff --git a/glx/glxcmds.c b/glx/glxcmds.c index 8d13c15..1e8044b 100644 --- a/glx/glxcmds.c +++ b/glx/glxcmds.c @@ -530,6 +530,7 @@ __glXGetDrawable(__GLXcontext *glxc, GLXDrawable drawId, ClientPtr client, *error = BadAlloc; return NULL; } + pGlxDraw->refcnt++; return pGlxDraw; } @@ -1102,6 +1103,7 @@ __glXDrawableInit(__GLXdrawable *drawable, drawable->drawId = drawId; drawable->config = config; drawable->eventMask = 0; + drawable->refcnt = 0; return GL_TRUE; } @@ -1131,14 +1133,17 @@ DoCreateGLXDrawable(ClientPtr client, __GLXscreen *pGlxScreen, pGlxDraw->destroy (pGlxDraw); return BadAlloc; } + pGlxDraw->refcnt++; /* 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 (drawableId != glxDrawableId && - !AddResource(pDraw->id, __glXDrawableRes, pGlxDraw)) { - pGlxDraw->destroy (pGlxDraw); - return BadAlloc; + if (drawableId != glxDrawableId) { + if (!AddResource(pDraw->id, __glXDrawableRes, pGlxDraw)) { + pGlxDraw->destroy (pGlxDraw); + return BadAlloc; + } + pGlxDraw->refcnt++; } return Success; diff --git a/glx/glxdrawable.h b/glx/glxdrawable.h index 2a365c5..e3d4bb5 100644 --- a/glx/glxdrawable.h +++ b/glx/glxdrawable.h @@ -51,6 +51,8 @@ struct __GLXdrawable { void (*waitX)(__GLXdrawable *); void (*waitGL)(__GLXdrawable *); + int refcnt; /* number of resources handles referencing this */ + DrawablePtr pDraw; XID drawId; diff --git a/glx/glxext.c b/glx/glxext.c index f5ebe4f..499567a 100644 --- a/glx/glxext.c +++ b/glx/glxext.c @@ -126,16 +126,9 @@ static Bool DrawableGone(__GLXdrawable *glxPriv, XID xid) { __GLXcontext *c, *next; - /* If this drawable was created using glx 1.3 drawable - * 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); - } + glxPriv->refcnt--; + if (glxPriv->refcnt > 0) + return True; for (c = glxAllContexts; c; c = next) { next = c->next;
On Don, 2010-12-16 at 11:54 -0500, Kristian Høgsberg wrote: > From: Chris Wilson <chris@chris-wilson.co.uk> > > 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. > > v2: Apply fixes and combine with another bug fix from Michel Dänzer, > https://bugs.freedesktop.org/show_bug.cgi?id=28181 > v3: Just use the refcnt and don't try to free other resources in the > DrawableGone callback. > > Signed-off-by: Kristian Høgsberg <krh@bitplanet.net> > > Can we just do this instead? Didn't test the patch, but this should do > the same and actually simplifies the code. > > Kristian > > --- > glx/glxcmds.c | 13 +++++++++---- > glx/glxdrawable.h | 2 ++ > glx/glxext.c | 13 +++---------- > 3 files changed, 14 insertions(+), 14 deletions(-) > > diff --git a/glx/glxcmds.c b/glx/glxcmds.c > index 8d13c15..1e8044b 100644 > --- a/glx/glxcmds.c > +++ b/glx/glxcmds.c > @@ -1131,14 +1133,17 @@ DoCreateGLXDrawable(ClientPtr client, __GLXscreen *pGlxScreen, > > /* 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 (drawableId != glxDrawableId && > - !AddResource(pDraw->id, __glXDrawableRes, pGlxDraw)) { > - pGlxDraw->destroy (pGlxDraw); > - return BadAlloc; > + if (drawableId != glxDrawableId) { > + if (!AddResource(pDraw->id, __glXDrawableRes, pGlxDraw)) { > + pGlxDraw->destroy (pGlxDraw); > + return BadAlloc; > + } > + pGlxDraw->refcnt++; The fixed version of this part from Chris' v2 patch is needed. > diff --git a/glx/glxext.c b/glx/glxext.c > index f5ebe4f..499567a 100644 > --- a/glx/glxext.c > +++ b/glx/glxext.c > @@ -126,16 +126,9 @@ static Bool DrawableGone(__GLXdrawable *glxPriv, XID xid) > { > __GLXcontext *c, *next; > > - /* If this drawable was created using glx 1.3 drawable > - * 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); > - } > + glxPriv->refcnt--; > + if (glxPriv->refcnt > 0) > + return True; If the IDs are different, doesn't this mean the GLX drawable will never actually be destroyed before the X drawable?