Bug 659842 - [topcrash] release the GL context before calling glXDestroyContext - r=karlt
authorBenoit Jacob <bjacob@mozilla.com>
Fri, 17 Jun 2011 11:49:27 -0400
changeset 71238 60182c83c9251a3af42cab9835bca41baf4b2fb7
parent 71237 9ac190a247ad0148909c05389079db1c1e379ff4
child 71239 e2d9e1039dca2b22447d260f17f39ff453c098ff
push id20513
push userbjacob@mozilla.com
push dateFri, 17 Jun 2011 16:04:46 +0000
treeherdermozilla-central@60182c83c925 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskarlt
bugs659842
milestone7.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 659842 - [topcrash] release the GL context before calling glXDestroyContext - r=karlt See the glXDestroyContext man page: If GLX rendering context ctx is not current to any thread, glXDestroyContext destroys it immediately. Otherwise, ctx is destroyed when it becomes not current to any thread. In either case, the resource ID referenced by ctx is freed immediately. In other words, if we want glXDestroyContext to have the well-defined semantics of destroying the context before future X commands take effect, we must first release the GL context before calling it. We were failing to do that, but we were destroying the drawable immediately after that call, and as a result, the context was outliving its underlying drawable. This eventually resulted in X_GLXMakeCurrent: GLXBadContextTag X errors on subsequent glXMakeCurrent calls.
gfx/thebes/GLContextProviderGLX.cpp
toolkit/xre/glxtest.cpp
--- a/gfx/thebes/GLContextProviderGLX.cpp
+++ b/gfx/thebes/GLContextProviderGLX.cpp
@@ -398,16 +398,21 @@ TRY_AGAIN_NO_SHARING:
 
         return glContext.forget();
     }
 
     ~GLContextGLX()
     {
         MarkDestroyed();
 
+        // see bug 659842 comment 76
+        bool success = sGLXLibrary.xMakeCurrent(mDisplay, None, nsnull);
+        NS_ABORT_IF_FALSE(success,
+            "glXMakeCurrent failed to release GL context before we call glXDestroyContext!");
+
         sGLXLibrary.xDestroyContext(mDisplay, mContext);
 
         if (mDeleteDrawable) {
             sGLXLibrary.xDestroyPixmap(mDisplay, mDrawable);
         }
     }
 
     GLContextType GetContextType() {
--- a/toolkit/xre/glxtest.cpp
+++ b/toolkit/xre/glxtest.cpp
@@ -207,18 +207,18 @@ static void glxtest()
   // as these would crash the application.
   XSync(dpy, False);
   
   ///// Finally write data to the pipe /////
   write(write_end_of_the_pipe, buf, length);
 
   ///// Clean up. Indeed, the parent process might fail to kill us (e.g. if it doesn't need to check GL info)
   ///// so we might be staying alive for longer than expected, so it's important to consume as little memory as
-  ///// possible.
-  glXDestroyContext(dpy, context);
+  ///// possible. Also we want to check that we're able to do that too without generating X errors.
+  glXMakeCurrent(dpy, None, NULL); // must release the GL context before destroying it
   glXDestroyPixmap(dpy, glxpixmap);
   XFreePixmap(dpy, pixmap);
   XCloseDisplay(dpy);
   dlclose(libgl);
 }
 
 /** \returns true in the child glxtest process, false in the parent process */
 bool fire_glxtest_process()