Bug 8443 - MakeContextCurrent may use the wrong opcode
Summary: MakeContextCurrent may use the wrong opcode
Alias: None
Product: Mesa
Classification: Unclassified
Component: GLX (show other bugs)
Version: git
Hardware: All All
: high normal
Assignee: mesa-dev
QA Contact:
Depends on:
Reported: 2006-09-27 15:54 UTC by Jamey Sharp
Modified: 2009-08-24 12:24 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:

Major clean up of MakeContextCurrent (6.55 KB, patch)
2006-10-05 16:19 UTC, Ian Romanick
Details | Splinter Review
Small test app for glXMakeCurrent (671 bytes, text/plain)
2006-10-14 00:41 UTC, Jesse Allen
currentContextTag patch version 1 (394 bytes, patch)
2006-10-16 08:18 UTC, Jesse Allen
Details | Splinter Review
currentContextTag patch version 2 (276 bytes, patch)
2006-10-16 08:19 UTC, Jesse Allen
Details | Splinter Review
Don't use currentContextTag from a direct context (634 bytes, patch)
2006-10-16 08:33 UTC, Ian Romanick
Details | Splinter Review

Description Jamey Sharp 2006-09-27 15:54:07 UTC
When a thread switches from one Display to another, I think glxext.c currently
uses the same major opcode for both Displays, because it uses the new Display to
get the old opcode:

    opcode = __glXSetupForCommand(dpy);
    oldOpcode = (gc == oldGC) ? opcode : __glXSetupForCommand(dpy);

I think the following patch fixes the problem:

--- glxext.c    26 Sep 2006 23:56:20 -0000      1.25
+++ glxext.c    27 Sep 2006 22:38:19 -0000
@@ -1582,7 +1582,7 @@ USED static Bool MakeContextCurrent(Disp
     oldGC = __glXGetCurrentContext();
-    oldOpcode = (gc == oldGC) ? opcode : __glXSetupForCommand(dpy);
+    oldOpcode = (gc == oldGC) ? opcode : __glXSetupForCommand(oldGC->currentDpy);
     if (!oldOpcode) {
        return GL_FALSE;
Comment 1 Brian Paul 2006-10-04 09:23:31 UTC
I've checked in your fix.  Thanks.
Comment 2 Ian Romanick 2006-10-05 14:22:36 UTC
That fix actually makes things worse.  On the first call to glXMakeCurrent, the
current context is set to &dummyContext.  dummyContext.currentDpy is NULL, so
the call to __glXSetupForCommand segfaults.

I added a "&& oldGC != &dummyContext" to the condition, and that makes the
segfault go away.  That routine is a great big mess, so I'm going to see if I
can clean it up a bit before I actually commit anything.

Watch this space for patches.
Comment 3 Ian Romanick 2006-10-05 16:19:53 UTC
Created attachment 7271 [details] [review]
Major clean up of MakeContextCurrent

Rearrange most of the internals of MakeContextCurrent.	Put all of the code to
bind the new context up front.	If that is successful, unbind the old context. 
This saves a lot of code and removes some locking crazyiness.

This patch has been tested for indirect rendering with glxinfo, glxgears,
manywin, and wincopy.
Comment 4 Brian Paul 2006-10-06 07:36:50 UTC
You might as well check this in, Ian, so that it gets exercised.
Comment 5 Ian Romanick 2006-10-06 19:52:56 UTC
Re-closing.  I committed a slightly modified version of patch #7271.
Comment 6 Jesse Allen 2006-10-14 00:21:23 UTC

I'm seeing a problem with this call:

pglXMakeCurrent(gdi_display, None, NULL);

X Error of failed request:  GLXBadContextTag
  Major opcode of failed request:  143 (GLX)
  Minor opcode of failed request:  5 (X_GLXMakeCurrent)
  Serial number of failed request:  25
  Current serial number in output stream:  25

It's found in the wine source tree under dlls/winex11.drv/opengl.c:308. It used
to work.
Comment 7 Jesse Allen 2006-10-14 00:41:09 UTC
Created attachment 7411 [details]
Small test app for glXMakeCurrent

This is a small test app showing my problem so you don't have to run Wine.
Comment 8 Jesse Allen 2006-10-16 08:18:31 UTC
Created attachment 7427 [details] [review]
currentContextTag patch version 1
Comment 9 Jesse Allen 2006-10-16 08:19:08 UTC
Created attachment 7428 [details] [review]
currentContextTag patch version 2
Comment 10 Ian Romanick 2006-10-16 08:33:20 UTC
Created attachment 7429 [details] [review]
Don't use currentContextTag from a direct context

-1 is used as the currentContextTag of direct rendering contexts to indicate
that the context does not have a valid currentContextTag from the point of view
of the server.	I believe the correct solution is to detect the oldGC->isDirect
case in the call to SendMakeCurrentRequest and use None as the old context tag.
 This patch implements that.

Please let me know if this works.
Comment 11 Jesse Allen 2006-10-16 09:24:53 UTC
It works thanks.
Comment 12 Adam Jackson 2009-08-24 12:24:32 UTC
Mass version move, cvs -> git

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.