[Mesa-dev] [PATCH] RE: Mesa garbage collection and threads

[Andy Skinner]

I want to submit our patch and see what people thought.  See the previous email with the same subject as above (with PATCH flag).

This change is intended to reduce the chances of XSync being called for a Display* on any old thread.  The garbage collection looks at every buffer, and when a Display* is available and the buffer is of the right type, etc, calls XSync on the Display*.  But XMesaGarbageCollect can be called from multiple threads.  We have a legacy situation where different Display pointers are kept on different threads.

Everywhere that XMesaGarbageCollect is called, all in fakeglx.c, there is a Display* available.  I'd like to pass that Display* in and only look at buffers whose Display* matches.

The change follows.  Our "git" doesn't have a send-email, so I just diffed the results to a file and am pasting.

The main feedback I want is about whether it is a problem that doing garbage collection when I'm dealing with Display A won't look at Display B.  If you lose a buffer associated with a Display, and don't do anything else with a buffer on that Display, I think this might leak that buffer.

thanks
andy


diff --git a/src/mesa/drivers/x11/xm_api.c b/src/mesa/drivers/x11/xm_api.c
index cb71b34..aad902d 100644
--- a/src/mesa/drivers/x11/xm_api.c
+++ b/src/mesa/drivers/x11/xm_api.c
@@ -2064,12 +2064,12 @@ void xmesa_destroy_buffers_on_display(XMesaDisplay *dpy)
  * Look for XMesaBuffers whose X window has been destroyed.
  * Deallocate any such XMesaBuffers.
  */
-void XMesaGarbageCollect( void )
+void XMesaGarbageCollect( XMesaDisplay* dpy )
 {
    XMesaBuffer b, next;
    for (b=XMesaBufferList; b; b=next) {
       next = b->Next;
-      if (b->display && b->frontxrb->drawable && b->type == WINDOW) {
+      if (b->display && b->display == dpy && b->frontxrb->drawable && b->type == WINDOW) {
          XSync(b->display, False);
          if (!window_exists( b->display, b->frontxrb->drawable )) {
             /* found a dead window, free the ancillary info */


diff --git a/src/mesa/drivers/x11/xmesa.h b/src/mesa/drivers/x11/xmesa.h
index 98737fa..347394e 100644
--- a/src/mesa/drivers/x11/xmesa.h
+++ b/src/mesa/drivers/x11/xmesa.h
@@ -324,7 +324,7 @@ extern const char *XMesaGetString( XMesaContext c, int name );
  *
  * New in Mesa 2.3.
  */
-extern void XMesaGarbageCollect( void );
+extern void XMesaGarbageCollect( XMesaDisplay* dpy );
 
 
 
diff --git a/src/mesa/drivers/x11/fakeglx.c b/src/mesa/drivers/x11/fakeglx.c
index 577e27d..48657b4 100644
--- a/src/mesa/drivers/x11/fakeglx.c
+++ b/src/mesa/drivers/x11/fakeglx.c
@@ -1292,7 +1292,7 @@ Fake_glXCreateContext( Display *dpy, XVisualInfo *visinfo,
 
    /* deallocate unused windows/buffers */
 #if 0
-   XMesaGarbageCollect();
+   XMesaGarbageCollect(dpy);
 #endif
 
    xmvis = find_glx_visual( dpy, visinfo );
@@ -1533,7 +1533,7 @@ Fake_glXDestroyContext( Display *dpy, GLXContext ctx )
    MakeCurrent_PrevDrawBuffer = 0;
    MakeCurrent_PrevReadBuffer = 0;
    XMesaDestroyContext( glxCtx->xmesaContext );
-   XMesaGarbageCollect();
+   XMesaGarbageCollect(dpy);
    free(glxCtx);
 }
 
@@ -2327,7 +2327,7 @@ Fake_glXCreateNewContext( Display *dpy, GLXFBConfig config,
       return 0;
 
    /* deallocate unused windows/buffers */
-   XMesaGarbageCollect();
+   XMesaGarbageCollect(dpy);
 
    glxCtx->xmesaContext = XMesaCreateContext(xmvis,
                                    shareCtx ? shareCtx->xmesaContext : NULL);
@@ -2542,7 +2542,7 @@ Fake_glXCreateContextWithConfigSGIX(Display *dpy, GLXFBConfigSGIX config, int re
       return 0;
 
    /* deallocate unused windows/buffers */
-   XMesaGarbageCollect();
+   XMesaGarbageCollect(dpy);
 
    glxCtx->xmesaContext = XMesaCreateContext(xmvis,
                                    shareCtx ? shareCtx->xmesaContext : NULL);