Re: [Mesa3d-dev] [rfc patch] dri drawable ref counting

2009-04-16 Thread Thomas Hellstrom 
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

Re: [Mesa3d-dev] [rfc patch] dri drawable ref counting

2009-04-16 Thread Michel Dänzer 
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

[Mesa3d-dev] [rfc patch] dri drawable ref counting

2009-04-15 Thread Dave Airlie 

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