Attachment 8488 Details for Bug 7205 – Clean stray framebuffer and glxPriv pointers in reused contexts

[patch] Clean stray framebuffer and glxPriv pointers in reused contexts

p_drawable_privclean.diff (text/plain), 6.01 KB, created by Matthias Hopf on 2007-01-23 11:49:57 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Matthias Hopf

Created: 2007-01-23 11:49:57 UTC

Size: 6.01 KB

patch

obsolete

>Novell Bugzilla #211314 and others
>Maybe Xorg Bugzilla #7205 and probably others
>
>
>Programs apparently crash if a context is reused for a window that changed in
>the meantime. Crashes apparently only occure for software rendering.
>
>The basic problem is that a buffer is freed, when the according window is
>closed, but the old context (which is no longer referenced in _glapi_Context
>or in the according glxPriv structures) still has a reference to this buffer,
>and is reactivated later. As the buffer points to a freed memory location, it
>is no longer recognized as an old buffer (because buffer->Name - by accident -
>contains 0x00cccccc) and not replaced.
>
>
>About this patch:
>
>The correct way to remove the reference would be to nuke it when a context is
>disassociated from a drawable - because after that only the Xserver itself
>(and not the Mesa subsystem) knows about the context which still has the stray
>buffer and glxPriv pointers.
>Unfortunately, if you do that, the server crashes in glxSwapBuffers() - I
>don't known enough about that code to decide, whether this is a broken
>implementation, a bug in glxSwapBuffers() (there might be other calls), broken
>by design, or deliberately chosen this way and actually correct.
>
>So the only way is to scan all contexts known for this client for references
>to this buffer (and the according private structure) when the buffer is
>actually destroyed. This is done in __glXMesaDrawableDestroy(), which has
>access to all needed data - all but one.
>
>Scanning all contexts only needs the client id (which is known), but the API
>call FindClientResourcesByType() actually needs a ClientPtr - just to extract
>the client id! The ClientPtr is only known in glxext.c in a static array
>__glXClients. I don't know of a different API to get the ClientPtr from, so I
>exported it. One other possibility would be to add a ClientPtr fetch call to
>glxext.c, another one to create a new FindClientIDResourcesByType(), another
>one to create a Client structure with just the id included (ugh!) just for
>this API call. Making the array extern was the simplest thing to do, but is
>not necessarily the right solution.
>
>There is probably has a better / right possibility to remove all pointers to
>the buffer (MakeCurrent2?) - but I don't have a better idea ATM.
>
>As I consider this a workaround / saveguard for some deeply involved buffer
>handling bug, I added a lot of ErrorF()s that indicate these failures. If
>removing these references at this point is the only valid solution, I'd gladly
>remove these. As the scanning is only invoked on buffer destroys and doesn't
>do a lot of scanning, performance is not an issue here.
>
>Matthias Hopf <mhopf@suse.de>
>
>
>--- GL/glx/glxext.c.orig	2007-01-23 16:00:19.000000000 +0100
>+++ GL/glx/glxext.c	2007-01-23 16:00:23.000000000 +0100
>@@ -59,7 +59,7 @@ xGLXSingleReply __glXReply;
> ** A set of state for each client.  The 0th one is unused because client
> ** indices start at 1, not 0.
> */
>-static __GLXclientState *__glXClients[MAXCLIENTS + 1];
>+__GLXclientState *__glXClients[MAXCLIENTS + 1];
> 
> /*
> ** Forward declarations.
>
>--- GL/mesa/X/xf86glx.c.orig	2007-01-23 12:48:28.000000000 +0100
>+++ GL/mesa/X/xf86glx.c	2007-01-23 19:37:23.000000000 +0100
>@@ -95,12 +95,70 @@ static XMesaVisual find_mesa_visual(__GL
> 
> 
> static void
>+__glXMesaDrawableDestoryCB(pointer baseCtx, XID id, pointer basePriv)
>+{
>+    __GLXMESAcontext  *context  = (__GLXMESAcontext *) baseCtx;
>+    __GLXdrawable     *glxPriv  = (__GLXdrawable *) basePriv;
>+    void              *buffer   =((__GLXMESAdrawable *) glxPriv) ->xm_buf;
>+    __GLXMESAdrawable *drawPriv, *readPriv;
>+
>+    if (context->base.drawPriv == glxPriv ||
>+	context->base.readPriv == glxPriv) {
>+      ErrorF ("Cleaning up stray priv pointers to %p in context %p\n",
>+	      glxPriv, context);
>+      if (context->base.drawPriv == glxPriv)
>+        context->base.drawPriv = NULL;
>+      if (context->base.readPriv == glxPriv)
>+        context->base.readPriv = NULL;
>+    }
>+    drawPriv = (__GLXMESAdrawable *) context->base.drawPriv;
>+    readPriv = (__GLXMESAdrawable *) context->base.readPriv;
>+    if ((drawPriv && drawPriv->xm_buf == buffer) ||
>+	(readPriv && readPriv->xm_buf == buffer)) {
>+      /* This should not happen, but the alternative is a sure server crash */
>+      ErrorF ("ERROR: Draw priv in context %p is not freed yet, "
>+	      "but has stray buffer pointer %p\n", context, buffer);
>+      if (drawPriv && drawPriv->xm_buf == buffer)
>+	drawPriv->xm_buf = NULL;
>+      if (readPriv && readPriv->xm_buf == buffer)
>+	readPriv->xm_buf = NULL;
>+    }
>+    if (context->xmesa &&
>+	(context->xmesa->mesa.DrawBuffer == buffer ||
>+	 context->xmesa->mesa.ReadBuffer == buffer ||
>+	 context->xmesa->mesa.WinSysDrawBuffer == buffer ||
>+	 context->xmesa->mesa.WinSysReadBuffer == buffer ||
>+	 context->xmesa->xm_buffer == buffer)) {
>+      ErrorF ("Cleaning up stray xmesa pointers to buffer %p in context %p\n",
>+	      buffer, context);
>+      if (context->xmesa->mesa.DrawBuffer == buffer)
>+        context->xmesa->mesa.DrawBuffer = NULL;
>+      if (context->xmesa->mesa.ReadBuffer == buffer)
>+        context->xmesa->mesa.ReadBuffer = NULL;
>+      if (context->xmesa->mesa.WinSysDrawBuffer == buffer)
>+        context->xmesa->mesa.WinSysDrawBuffer = NULL;
>+      if (context->xmesa->mesa.WinSysReadBuffer == buffer)
>+        context->xmesa->mesa.WinSysReadBuffer = NULL;
>+      if (context->xmesa->xm_buffer == buffer)
>+        context->xmesa->xm_buffer = NULL;
>+    }
>+}
>+
>+extern __GLXclientState *__glXClients[];
>+static void
> __glXMesaDrawableDestroy(__GLXdrawable *base)
> {
>     __GLXMESAdrawable *glxPriv = (__GLXMESAdrawable *) base;
>+    ClientPtr client = __glXClients[CLIENT_ID(base->drawId)]->client;
> 
>-    if (glxPriv->xm_buf != NULL)
>+    if (glxPriv->xm_buf != NULL) {
>+      /* There may still be stray pointers in contexts that are no longer
>+       * visible to Mesa. Scan all contexts for this client and nuke those
>+       * pointers */
>+      FindClientResourcesByType(client, __glXContextRes,
>+				__glXMesaDrawableDestoryCB, glxPriv);
>       XMesaDestroyBuffer(glxPriv->xm_buf);
>+    }
>     xfree(glxPriv);
> }
>

Actions: View | Splinter Review

Attachments on bug 7205: 5893 | 8257 | 8258 | 8487 | 8488 | 8501 | 8884

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.