Changeset 39507 in vbox for trunk/src/VBox/GuestHost/OpenGL

Timestamp:

Dec 2, 2011 7:44:16 AM (13 years ago)

Author:

vboxsync

Message:

crOpenGL: fix destroyed context refference in TLS

Location:

trunk/src/VBox/GuestHost/OpenGL

Files:

• include/cr_glstate.h (modified) (2 diffs)
• state_tracker/state.h (modified) (2 diffs)
• state_tracker/state_glsl.c (modified) (2 diffs)
• state_tracker/state_init.c (modified) (9 diffs)

trunk/src/VBox/GuestHost/OpenGL/include/cr_glstate.h

@@ -118 +118 @@
 struct CRContext {
     int id;
+
+#ifdef CHROMIUM_THREADSAFE
+    /* we keep reference counting of context's makeCurrent for different threads
+     * this is primarily needed to avoid having an invalid memory reference in the TLS
+     * when the context is assigned to more than one threads and then destroyed from
+     * one of those, i.e.
+     * 1. Thread1 -> MakeCurrent(ctx1);
+     * 2. Thread2 -> MakeCurrent(ctx1);
+     * 3. Thread1 -> Destroy(ctx1);
+     * => Thread2 still refers to destroyed ctx1
+     * */
+    /* number of threads that have context set as current */
+    volatile int cRefs;
+#endif
+
     CRbitvalue bitid[CR_MAX_BITARRAY];
     CRbitvalue neg_bitid[CR_MAX_BITARRAY];
@@ -181 +196 @@
 DECLEXPORT(void) crStateInit(void);
 DECLEXPORT(void) crStateDestroy(void);
+DECLEXPORT(void) crStateOnThreadAttachDetach(GLboolean attach);
 DECLEXPORT(CRContext *) crStateCreateContext(const CRLimitsState *limits, GLint visBits, CRContext *share);
 DECLEXPORT(CRContext *) crStateCreateContextEx(const CRLimitsState *limits, GLint visBits, CRContext *share, GLint presetID);

trunk/src/VBox/GuestHost/OpenGL/state_tracker/state.h

@@ -11 +11 @@
 #ifdef CHROMIUM_THREADSAFE
 #include "cr_threads.h"
+#include <iprt/asm.h>
 #endif
 
@@ -25 +26 @@
 extern CRtsd __contextTSD;
 #define GetCurrentContext() (CRContext *) crGetTSD(&__contextTSD)
+
+/* NOTE: below ref & SetCurrentContext stuff is supposed to be used only internally!!
+ * it is placed here only to simplify things since some code besides state_init.c
+ * (i.e. state_glsl.c) is using it */
+void crStateFreeContext(CRContext *ctx);
+#define CRCONTEXT_ADDREF(_ctx) do { \
+        int cRefs = ASMAtomicIncS32(&((CRContext*)(_ctx))->cRefs); \
+        CRASSERT(cRefs > 1); \
+    } while (0)
+#define CRCONTEXT_RELEASE(_ctx) do { \
+        int cRefs = ASMAtomicDecS32(&((CRContext*)(_ctx))->cRefs); \
+        CRASSERT(cRefs >= 0); \
+        if (!cRefs) { \
+            crStateFreeContext((_ctx)); \
+        } \
+    } while (0)
+#define SetCurrentContext(_ctx) do { \
+        CRContext * oldCur = GetCurrentContext(); \
+        if (oldCur != (_ctx)) { \
+            if (oldCur) { \
+                CRCONTEXT_RELEASE(oldCur); \
+            } \
+            if ((_ctx)) { \
+                CRCONTEXT_ADDREF(_ctx); \
+            } \
+            crSetTSD(&__contextTSD, _ctx); \
+        } \
+    } while (0)
 #else
 extern CRContext *__currentContext;

trunk/src/VBox/GuestHost/OpenGL/state_tracker/state_glsl.c

@@ -174 +174 @@
       as the current context isn't the one being destroyed*/
 #ifdef CHROMIUM_THREADSAFE
-    crSetTSD(&__contextTSD, ctx);
+    CRASSERT(g != ctx);
+    CRASSERT(!ctx->cRefs);
+    ++ctx->cRefs; /* <- this is a hack to avoid subsequent SetCurrentContext(g) do recursive Destroy for ctx */
+    if (g)
+        CRCONTEXT_ADDREF(g); /* <- ensure the g is not destroyed by the following SetCurrentContext call */
+    SetCurrentContext(ctx);
 #else
     __currentContext = ctx;
@@ -183 +188 @@
 
 #ifdef CHROMIUM_THREADSAFE
-    crSetTSD(&__contextTSD, g);
+    SetCurrentContext(g);
+    if (g)
+        CRCONTEXT_RELEASE(g);
+    --ctx->cRefs; /* <- restore back the cRefs (see above) */
 #else
     __currentContext = g;

trunk/src/VBox/GuestHost/OpenGL/state_tracker/state_init.c

@@ -158 +158 @@
 
     ctx->id = i;
+#ifdef CHROMIUM_THREADSAFE
+    ctx->cRefs = 1;
+#endif
     ctx->flush_func = NULL;
     for (j=0;j<CR_MAX_BITARRAY;j++){
@@ -258 +261 @@
 
 /*@todo crStateAttribDestroy*/
-static void
+void
 crStateFreeContext(CRContext *ctx)
 {
@@ -307 +310 @@
         /* Free the default/NULL context.
          * Ensures context bits are reset */
+#ifdef CHROMIUM_THREADSAFE
+        SetCurrentContext(NULL);
+        CRCONTEXT_RELEASE(defaultContext);
+#else
         crStateFreeContext(defaultContext);
-#ifdef CHROMIUM_THREADSAFE
-        crSetTSD(&__contextTSD, NULL);
-#else
         __currentContext = NULL;
 #endif
@@ -324 +328 @@
 
 #ifdef CHROMIUM_THREADSAFE
-    crSetTSD(&__contextTSD, defaultContext);
+    SetCurrentContext(defaultContext);
 #else
     __currentContext = defaultContext;
@@ -434 +438 @@
             crStateSwitchContext(current, defaultContext);
 #ifdef CHROMIUM_THREADSAFE
-        crSetTSD(&__contextTSD, defaultContext);
+        SetCurrentContext(defaultContext);
 #else
         __currentContext = defaultContext;
@@ -443 +447 @@
     g_availableContexts[ctx->id] = 0;
 
+#ifdef CHROMIUM_THREADSAFE
+    CRCONTEXT_RELEASE(ctx);
+#else
     crStateFreeContext(ctx);
+#endif
 }
 
@@ -467 +475 @@
 
 #ifdef CHROMIUM_THREADSAFE
-    crSetTSD(&__contextTSD, ctx);
+    SetCurrentContext(ctx);
 #else
     __currentContext = ctx;
@@ -493 +501 @@
 
 #ifdef CHROMIUM_THREADSAFE
-    crSetTSD(&__contextTSD, ctx);
+    SetCurrentContext(ctx);
 #else
     __currentContext = ctx;
@@ -549 +557 @@
 }
 
+void crStateOnThreadAttachDetach(GLboolean attach)
+{
+    if (attach)
+        return;
+
+    /* release the context ref so that it can be freed */
+    SetCurrentContext(NULL);
+}