Re: [Mesa3d-dev] [rfc patch] dri drawable ref counting
On Thu, 2009-04-16 at 04:24 +0100, Dave Airlie wrote: > valgrind was showing a race between the drawable getting destroyed > by the X resource freeing code, and the context getting destroyed > later and freeing the drawable. Does that still happen with Kristian's GLX drawable private leak fix? > However I've no idea if some other combination of things could cause > this code to leak. Though indeed I've been wondering if Kristian's fix couldn't cause problems of its own due to just NULLing out the context drawable pointers in DrawableGone() but not doing the flushing etc. done in DoMakeCurrent(). -- Earthling Michel Dänzer |http://www.vmware.com Libre software enthusiast | Debian, X and DRI developer -- Stay on top of everything new and different, both inside and around Java (TM) technology - register by April 22, and save $200 on the JavaOne (SM) conference, June 2-5, 2009, San Francisco. 300 plus technical and hands-on sessions. Register today. Use priority code J9JMT32. http://p.sf.net/sfu/p ___ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
Re: [Mesa3d-dev] [rfc patch] dri drawable ref counting
Dave Airlie wrote: > valgrind was showing a race between the drawable getting destroyed > by the X resource freeing code, and the context getting destroyed > later and freeing the drawable. > > However I've no idea if some other combination of things could cause > this code to leak. > > Any one else have any ideas? > Looks good to me, Although the refcount should probably be atomic or protected by a mutex. /Thomas > --- > src/mesa/drivers/dri/common/dri_util.c | 32 > > 1 files changed, 24 insertions(+), 8 deletions(-) > > diff --git a/src/mesa/drivers/dri/common/dri_util.c > b/src/mesa/drivers/dri/common/dri_util.c > index 38c2e7b..0ec4adc 100644 > --- a/src/mesa/drivers/dri/common/dri_util.c > +++ b/src/mesa/drivers/dri/common/dri_util.c > @@ -37,6 +37,9 @@ > typedef GLboolean ( * PFNGLXGETMSCRATEOMLPROC) (__DRIdrawable *drawable, > int32_t *numerator, int32_t *denominator); > #endif > > +static void dri_get_drawable(__DRIdrawable *pdp); > +static void dri_put_drawable(__DRIdrawable *pdp); > + > /** > * This is just a token extension used to signal that the driver > * supports setting a read drawable. > @@ -127,7 +130,7 @@ static int driUnbindContext(__DRIcontext *pcp) > return GL_FALSE; > } > > -pdp->refcount--; > +dri_put_drawable(pdp); > > if (prp != pdp) { > if (prp->refcount == 0) { > @@ -135,7 +138,7 @@ static int driUnbindContext(__DRIcontext *pcp) > return GL_FALSE; > } > > - prp->refcount--; > + dri_put_drawable(prp); > } > > > @@ -170,10 +173,10 @@ static int driBindContext(__DRIcontext *pcp, > pcp->driReadablePriv = prp; > if (pdp) { > pdp->driContextPriv = pcp; > - pdp->refcount++; > + dri_get_drawable(pdp); > } > if ( prp && pdp != prp ) { > - prp->refcount++; > + dri_get_drawable(prp); > } > } > > @@ -430,7 +433,7 @@ driCreateNewDrawable(__DRIscreen *psp, const __DRIconfig > *config, > > pdp->loaderPrivate = data; > pdp->hHWDrawable = hwDrawable; > -pdp->refcount = 0; > +pdp->refcount = 1; > pdp->pStamp = NULL; > pdp->lastStamp = 0; > pdp->index = 0; > @@ -483,12 +486,19 @@ dri2CreateNewDrawable(__DRIscreen *screen, > return pdraw; > } > > - > -static void > -driDestroyDrawable(__DRIdrawable *pdp) > +static void dri_get_drawable(__DRIdrawable *pdp) > +{ > +pdp->refcount++; > +} > + > +static void dri_put_drawable(__DRIdrawable *pdp) > { > __DRIscreenPrivate *psp; > > +pdp->refcount--; > +if (pdp->refcount) > + return; > + > if (pdp) { > psp = pdp->driScreenPriv; > (*psp->DriverAPI.DestroyBuffer)(pdp); > @@ -504,6 +514,12 @@ driDestroyDrawable(__DRIdrawable *pdp) > } > } > > +static void > +driDestroyDrawable(__DRIdrawable *pdp) > +{ > +dri_put_drawable(pdp); > +} > + > /*...@}*/ > > > -- Stay on top of everything new and different, both inside and around Java (TM) technology - register by April 22, and save $200 on the JavaOne (SM) conference, June 2-5, 2009, San Francisco. 300 plus technical and hands-on sessions. Register today. Use priority code J9JMT32. http://p.sf.net/sfu/p ___ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev
[Mesa3d-dev] [rfc patch] dri drawable ref counting
valgrind was showing a race between the drawable getting destroyed by the X resource freeing code, and the context getting destroyed later and freeing the drawable. However I've no idea if some other combination of things could cause this code to leak. Any one else have any ideas? --- src/mesa/drivers/dri/common/dri_util.c | 32 1 files changed, 24 insertions(+), 8 deletions(-) diff --git a/src/mesa/drivers/dri/common/dri_util.c b/src/mesa/drivers/dri/common/dri_util.c index 38c2e7b..0ec4adc 100644 --- a/src/mesa/drivers/dri/common/dri_util.c +++ b/src/mesa/drivers/dri/common/dri_util.c @@ -37,6 +37,9 @@ typedef GLboolean ( * PFNGLXGETMSCRATEOMLPROC) (__DRIdrawable *drawable, int32_t *numerator, int32_t *denominator); #endif +static void dri_get_drawable(__DRIdrawable *pdp); +static void dri_put_drawable(__DRIdrawable *pdp); + /** * This is just a token extension used to signal that the driver * supports setting a read drawable. @@ -127,7 +130,7 @@ static int driUnbindContext(__DRIcontext *pcp) return GL_FALSE; } -pdp->refcount--; +dri_put_drawable(pdp); if (prp != pdp) { if (prp->refcount == 0) { @@ -135,7 +138,7 @@ static int driUnbindContext(__DRIcontext *pcp) return GL_FALSE; } - prp->refcount--; + dri_put_drawable(prp); } @@ -170,10 +173,10 @@ static int driBindContext(__DRIcontext *pcp, pcp->driReadablePriv = prp; if (pdp) { pdp->driContextPriv = pcp; - pdp->refcount++; + dri_get_drawable(pdp); } if ( prp && pdp != prp ) { - prp->refcount++; + dri_get_drawable(prp); } } @@ -430,7 +433,7 @@ driCreateNewDrawable(__DRIscreen *psp, const __DRIconfig *config, pdp->loaderPrivate = data; pdp->hHWDrawable = hwDrawable; -pdp->refcount = 0; +pdp->refcount = 1; pdp->pStamp = NULL; pdp->lastStamp = 0; pdp->index = 0; @@ -483,12 +486,19 @@ dri2CreateNewDrawable(__DRIscreen *screen, return pdraw; } - -static void -driDestroyDrawable(__DRIdrawable *pdp) +static void dri_get_drawable(__DRIdrawable *pdp) +{ +pdp->refcount++; +} + +static void dri_put_drawable(__DRIdrawable *pdp) { __DRIscreenPrivate *psp; +pdp->refcount--; +if (pdp->refcount) + return; + if (pdp) { psp = pdp->driScreenPriv; (*psp->DriverAPI.DestroyBuffer)(pdp); @@ -504,6 +514,12 @@ driDestroyDrawable(__DRIdrawable *pdp) } } +static void +driDestroyDrawable(__DRIdrawable *pdp) +{ +dri_put_drawable(pdp); +} + /*...@}*/ -- 1.6.2.2 -- Stay on top of everything new and different, both inside and around Java (TM) technology - register by April 22, and save $200 on the JavaOne (SM) conference, June 2-5, 2009, San Francisco. 300 plus technical and hands-on sessions. Register today. Use priority code J9JMT32. http://p.sf.net/sfu/p ___ Mesa3d-dev mailing list Mesa3d-dev@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/mesa3d-dev