[Mesa-dev] [PATCH 18/21] mesa: protect the debug state with a mutex

Chia-I Wu olvaffe at gmail.com
Tue Apr 22 01:58:33 PDT 2014


When GL_DEBUG_OUTPUT_SYNCHRONOUS is GL_TRUE, drivers are allowed to log debug
messages from other threads.  The debug state needs to be protected.  We are
about to change mesa to spawn threads for deferred glCompileShader calls and
we will need this groundwork.

Signed-off-by: Chia-I Wu <olv at lunarg.com>
---
 src/mesa/drivers/dri/common/dri_util.c |   3 +-
 src/mesa/main/enable.c                 |   6 +-
 src/mesa/main/errors.c                 | 163 +++++++++++++++++++++++++--------
 src/mesa/main/errors.h                 |   5 +-
 src/mesa/main/get.c                    |  30 ++++--
 src/mesa/main/getstring.c              |  20 +++-
 src/mesa/main/mtypes.h                 |   1 +
 src/mesa/state_tracker/st_manager.c    |   3 +-
 8 files changed, 178 insertions(+), 53 deletions(-)

diff --git a/src/mesa/drivers/dri/common/dri_util.c b/src/mesa/drivers/dri/common/dri_util.c
index aed73c7..c085a21 100644
--- a/src/mesa/drivers/dri/common/dri_util.c
+++ b/src/mesa/drivers/dri/common/dri_util.c
@@ -449,9 +449,10 @@ driContextSetFlags(struct gl_context *ctx, uint32_t flags)
     if ((flags & __DRI_CTX_FLAG_FORWARD_COMPATIBLE) != 0)
         ctx->Const.ContextFlags |= GL_CONTEXT_FLAG_FORWARD_COMPATIBLE_BIT;
     if ((flags & __DRI_CTX_FLAG_DEBUG) != 0) {
-        struct gl_debug_state *debug = _mesa_get_debug_state(ctx);
+        struct gl_debug_state *debug = _mesa_lock_debug_state(ctx);
         if (debug) {
             debug->DebugOutput = GL_TRUE;
+            _mesa_unlock_debug_state(ctx);
         }
         ctx->Const.ContextFlags |= GL_CONTEXT_FLAG_DEBUG_BIT;
     }
diff --git a/src/mesa/main/enable.c b/src/mesa/main/enable.c
index edd4751..23e2239 100644
--- a/src/mesa/main/enable.c
+++ b/src/mesa/main/enable.c
@@ -372,9 +372,10 @@ _mesa_set_enable(struct gl_context *ctx, GLenum cap, GLboolean state)
             goto invalid_enum_error;
          }
          else {
-            struct gl_debug_state *debug = _mesa_get_debug_state(ctx);
+            struct gl_debug_state *debug = _mesa_lock_debug_state(ctx);
             if (debug) {
                debug->DebugOutput = state;
+               _mesa_unlock_debug_state(ctx);
             }
          }
          break;
@@ -383,9 +384,10 @@ _mesa_set_enable(struct gl_context *ctx, GLenum cap, GLboolean state)
             goto invalid_enum_error;
          }
          else {
-            struct gl_debug_state *debug = _mesa_get_debug_state(ctx);
+            struct gl_debug_state *debug = _mesa_lock_debug_state(ctx);
             if (debug) {
                debug->SyncOutput = state;
+               _mesa_unlock_debug_state(ctx);
             }
          }
          break;
diff --git a/src/mesa/main/errors.c b/src/mesa/main/errors.c
index b9f8fc6..89ac2c0 100644
--- a/src/mesa/main/errors.c
+++ b/src/mesa/main/errors.c
@@ -599,22 +599,87 @@ debug_pop_group(struct gl_debug_state *debug)
 
 
 /**
- * Return debug state for the context.  The debug state will be allocated
- * and initialized upon the first call.
+ * Lock and return debug state for the context.  The debug state will be
+ * allocated and initialized upon the first call.  If the allocation fails,
+ * return NULL and the debug state is not locked.
  */
 struct gl_debug_state *
-_mesa_get_debug_state(struct gl_context *ctx)
+_mesa_lock_debug_state(struct gl_context *ctx)
 {
+   mtx_lock(&ctx->DebugMutex);
+
    if (!ctx->Debug) {
       ctx->Debug = debug_create();
       if (!ctx->Debug) {
-         _mesa_error(ctx, GL_OUT_OF_MEMORY, "allocating debug state");
+         /*
+          * This function may be called from other threads.  When that is the
+          * case, we cannot record this OOM error.
+          */
+         GET_CURRENT_CONTEXT(cur);
+         bool can_record = (cur == ctx);
+
+         mtx_unlock(&ctx->DebugMutex);
+
+         if (can_record)
+            _mesa_error(ctx, GL_OUT_OF_MEMORY, "allocating debug state");
+
+         return NULL;
       }
    }
 
    return ctx->Debug;
 }
 
+void
+_mesa_unlock_debug_state(struct gl_context *ctx)
+{
+   mtx_unlock(&ctx->DebugMutex);
+}
+
+/**
+ * Call the debug callback.
+ */
+static void
+log_msg_call_locked(struct gl_context *ctx,
+                    enum mesa_debug_source source,
+                    enum mesa_debug_type type, GLuint id,
+                    enum mesa_debug_severity severity,
+                    GLint len, const char *buf,
+                    bool unlock)
+{
+   GLenum gl_source = debug_source_enums[source];
+   GLenum gl_type = debug_type_enums[type];
+   GLenum gl_severity = debug_severity_enums[severity];
+   GLDEBUGPROC callback = ctx->Debug->Callback;
+   const void *data = ctx->Debug->CallbackData;
+
+   if (unlock)
+      _mesa_unlock_debug_state(ctx);
+
+   callback(gl_source, gl_type, id, gl_severity, len, buf, data);
+}
+
+/**
+ * Log a client group message.
+ */
+static void
+log_msg_group_locked(struct gl_context *ctx,
+                     enum mesa_debug_source source,
+                     enum mesa_debug_type type, GLuint id,
+                     enum mesa_debug_severity severity,
+                     GLint len, const char *buf)
+{
+   struct gl_debug_state *debug = ctx->Debug;
+
+   if (debug_is_message_filtered(debug, source, type, id, severity))
+      return;
+
+   /* we will have to call the callback with the mutex locked */
+   if (ctx->Debug->Callback)
+      log_msg_call_locked(ctx, source, type, id, severity, len, buf, false);
+   else
+      debug_log_message(ctx->Debug, source, type, id, severity, len, buf);
+}
 
 /**
  * Log a client or driver debug message.
@@ -624,24 +689,28 @@ log_msg(struct gl_context *ctx, enum mesa_debug_source source,
         enum mesa_debug_type type, GLuint id,
         enum mesa_debug_severity severity, GLint len, const char *buf)
 {
-   struct gl_debug_state *debug = _mesa_get_debug_state(ctx);
+   struct gl_debug_state *debug = _mesa_lock_debug_state(ctx);
 
    if (!debug)
       return;
 
-   if (debug_is_message_filtered(debug, source, type, id, severity))
-      return;
-
-   if (debug->Callback) {
-       GLenum gl_type = debug_type_enums[type];
-       GLenum gl_severity = debug_severity_enums[severity];
-
-      debug->Callback(debug_source_enums[source], gl_type, id, gl_severity,
-                      len, buf, debug->CallbackData);
+   if (debug_is_message_filtered(debug, source, type, id, severity)) {
+      _mesa_unlock_debug_state(ctx);
       return;
    }
 
-   debug_log_message(debug, source, type, id, severity, len, buf);
+   /*
+    * When ctx->Debug->SyncOutput is GL_FALSE, the client is prepared for
+    * unsynchronous calls.  When it is GL_TRUE, we will not spawn threads.  In
+    * either case, we can call the callback unlocked.
+    */
+   if (ctx->Debug->Callback) {
+      log_msg_call_locked(ctx, source, type, id, severity, len, buf, true);
+   }
+   else {
+      debug_log_message(debug, source, type, id, severity, len, buf);
+      _mesa_unlock_debug_state(ctx);
+   }
 }
 
 
@@ -782,7 +851,7 @@ _mesa_GetDebugMessageLog(GLuint count, GLsizei logSize, GLenum *sources,
       return 0;
    }
 
-   debug = _mesa_get_debug_state(ctx);
+   debug = _mesa_lock_debug_state(ctx);
    if (!debug)
       return 0;
 
@@ -819,6 +888,8 @@ _mesa_GetDebugMessageLog(GLuint count, GLsizei logSize, GLenum *sources,
       debug_delete_messages(debug, 1);
    }
 
+   _mesa_unlock_debug_state(ctx);
+
    return ret;
 }
 
@@ -855,7 +926,7 @@ _mesa_DebugMessageControl(GLenum gl_source, GLenum gl_type,
       return;
    }
 
-   debug = _mesa_get_debug_state(ctx);
+   debug = _mesa_lock_debug_state(ctx);
    if (!debug)
       return;
 
@@ -867,6 +938,8 @@ _mesa_DebugMessageControl(GLenum gl_source, GLenum gl_type,
    else {
       debug_set_default_filter(debug, source, type, severity, enabled);
    }
+
+   _mesa_unlock_debug_state(ctx);
 }
 
 
@@ -874,10 +947,11 @@ void GLAPIENTRY
 _mesa_DebugMessageCallback(GLDEBUGPROC callback, const void *userParam)
 {
    GET_CURRENT_CONTEXT(ctx);
-   struct gl_debug_state *debug = _mesa_get_debug_state(ctx);
+   struct gl_debug_state *debug = _mesa_lock_debug_state(ctx);
    if (debug) {
       debug->Callback = callback;
       debug->CallbackData = userParam;
+      _mesa_unlock_debug_state(ctx);
    }
 }
 
@@ -887,18 +961,10 @@ _mesa_PushDebugGroup(GLenum source, GLuint id, GLsizei length,
                      const GLchar *message)
 {
    GET_CURRENT_CONTEXT(ctx);
-   struct gl_debug_state *debug = _mesa_get_debug_state(ctx);
    const char *callerstr = "glPushDebugGroup";
+   struct gl_debug_state *debug;
    struct gl_debug_msg *emptySlot;
 
-   if (!debug)
-      return;
-
-   if (debug->GroupStackDepth >= MAX_DEBUG_GROUP_STACK_DEPTH-1) {
-      _mesa_error(ctx, GL_STACK_OVERFLOW, "%s", callerstr);
-      return;
-   }
-
    switch(source) {
    case GL_DEBUG_SOURCE_APPLICATION:
    case GL_DEBUG_SOURCE_THIRD_PARTY:
@@ -915,10 +981,20 @@ _mesa_PushDebugGroup(GLenum source, GLuint id, GLsizei length,
    if (length < 0)
       length = strlen(message);
 
-   log_msg(ctx, gl_enum_to_debug_source(source),
-           MESA_DEBUG_TYPE_PUSH_GROUP, id,
-           MESA_DEBUG_SEVERITY_NOTIFICATION, length,
-           message);
+   debug = _mesa_lock_debug_state(ctx);
+   if (!debug)
+      return;
+
+   if (debug->GroupStackDepth >= MAX_DEBUG_GROUP_STACK_DEPTH-1) {
+      _mesa_unlock_debug_state(ctx);
+      _mesa_error(ctx, GL_STACK_OVERFLOW, "%s", callerstr);
+      return;
+   }
+
+   log_msg_group_locked(ctx, gl_enum_to_debug_source(source),
+                        MESA_DEBUG_TYPE_PUSH_GROUP, id,
+                        MESA_DEBUG_SEVERITY_NOTIFICATION, length,
+                        message);
 
    /* pop reuses the message details from push so we store this */
    emptySlot = debug_get_group_message(debug);
@@ -930,6 +1006,8 @@ _mesa_PushDebugGroup(GLenum source, GLuint id, GLsizei length,
                        length, message);
 
    debug_push_group(debug);
+
+   _mesa_unlock_debug_state(ctx);
 }
 
 
@@ -937,14 +1015,16 @@ void GLAPIENTRY
 _mesa_PopDebugGroup(void)
 {
    GET_CURRENT_CONTEXT(ctx);
-   struct gl_debug_state *debug = _mesa_get_debug_state(ctx);
    const char *callerstr = "glPopDebugGroup";
+   struct gl_debug_state *debug;
    struct gl_debug_msg *gdmessage;
 
+   debug = _mesa_lock_debug_state(ctx);
    if (!debug)
       return;
 
    if (debug->GroupStackDepth <= 0) {
+      _mesa_unlock_debug_state(ctx);
       _mesa_error(ctx, GL_STACK_UNDERFLOW, "%s", callerstr);
       return;
    }
@@ -952,20 +1032,22 @@ _mesa_PopDebugGroup(void)
    debug_pop_group(debug);
 
    gdmessage = debug_get_group_message(debug);
-   log_msg(ctx, gdmessage->source,
-           MESA_DEBUG_TYPE_POP_GROUP,
-           gdmessage->id,
-           MESA_DEBUG_SEVERITY_NOTIFICATION,
-           gdmessage->length, gdmessage->message);
+   log_msg_group_locked(ctx, gdmessage->source,
+                        MESA_DEBUG_TYPE_POP_GROUP,
+                        gdmessage->id,
+                        MESA_DEBUG_SEVERITY_NOTIFICATION,
+                        gdmessage->length, gdmessage->message);
 
    debug_clear_message(debug, gdmessage);
+
+   _mesa_unlock_debug_state(ctx);
 }
 
 
 void
 _mesa_init_errors(struct gl_context *ctx)
 {
-   /* no-op */
+   mtx_init(&ctx->DebugMutex, mtx_plain);
 }
 
 
@@ -977,6 +1059,8 @@ _mesa_free_errors_data(struct gl_context *ctx)
       /* set to NULL just in case it is used before context is completely gone. */
       ctx->Debug = NULL;
    }
+
+   mtx_destroy(&ctx->DebugMutex);
 }
 
 
@@ -1191,6 +1275,8 @@ _mesa_error( struct gl_context *ctx, GLenum error, const char *fmtString, ... )
    debug_get_id(&error_msg_id);
 
    do_output = should_output(ctx, error, fmtString);
+
+   mtx_lock(&ctx->DebugMutex);
    if (ctx->Debug) {
       do_log = !debug_is_message_filtered(ctx->Debug,
                                           MESA_DEBUG_SOURCE_API,
@@ -1201,6 +1287,7 @@ _mesa_error( struct gl_context *ctx, GLenum error, const char *fmtString, ... )
    else {
       do_log = GL_FALSE;
    }
+   mtx_unlock(&ctx->DebugMutex);
 
    if (do_output || do_log) {
       char s[MAX_DEBUG_MESSAGE_LENGTH], s2[MAX_DEBUG_MESSAGE_LENGTH];
diff --git a/src/mesa/main/errors.h b/src/mesa/main/errors.h
index e0706e5..2706a32 100644
--- a/src/mesa/main/errors.h
+++ b/src/mesa/main/errors.h
@@ -84,7 +84,10 @@ _mesa_gl_debug(struct gl_context *ctx,
 } while (0)
 
 struct gl_debug_state *
-_mesa_get_debug_state(struct gl_context *ctx);
+_mesa_lock_debug_state(struct gl_context *ctx);
+
+void
+_mesa_unlock_debug_state(struct gl_context *ctx);
 
 extern void
 _mesa_shader_debug(struct gl_context *ctx, GLenum type, GLuint *id,
diff --git a/src/mesa/main/get.c b/src/mesa/main/get.c
index 6d95790..0fde047 100644
--- a/src/mesa/main/get.c
+++ b/src/mesa/main/get.c
@@ -990,20 +990,38 @@ find_custom_value(struct gl_context *ctx, const struct value_desc *d, union valu
    /* GL_KHR_DEBUG */
    case GL_DEBUG_LOGGED_MESSAGES:
       {
-         struct gl_debug_state *debug = _mesa_get_debug_state(ctx);
-         v->value_int = debug ? debug->NumMessages : 0;
+         struct gl_debug_state *debug = _mesa_lock_debug_state(ctx);
+         if (debug) {
+            v->value_int = debug->NumMessages;
+            _mesa_unlock_debug_state(ctx);
+         }
+         else {
+            v->value_int = 0;
+         }
       }
       break;
    case GL_DEBUG_NEXT_LOGGED_MESSAGE_LENGTH:
       {
-         struct gl_debug_state *debug = _mesa_get_debug_state(ctx);
-         v->value_int = debug ? debug->NextMsgLength : 0;
+         struct gl_debug_state *debug = _mesa_lock_debug_state(ctx);
+         if (debug) {
+            v->value_int = debug->NextMsgLength;
+            _mesa_unlock_debug_state(ctx);
+         }
+         else {
+            v->value_int = 0;
+         }
       }
       break;
    case GL_DEBUG_GROUP_STACK_DEPTH:
       {
-         struct gl_debug_state *debug = _mesa_get_debug_state(ctx);
-         v->value_int = debug ? debug->GroupStackDepth : 0;
+         struct gl_debug_state *debug = _mesa_lock_debug_state(ctx);
+         if (debug) {
+            v->value_int = debug->GroupStackDepth;
+            _mesa_unlock_debug_state(ctx);
+         }
+         else {
+            v->value_int = 0;
+         }
       }
       break;
 
diff --git a/src/mesa/main/getstring.c b/src/mesa/main/getstring.c
index b0bd319..2fd4c1a 100644
--- a/src/mesa/main/getstring.c
+++ b/src/mesa/main/getstring.c
@@ -257,8 +257,14 @@ _mesa_GetPointerv( GLenum pname, GLvoid **params )
             goto invalid_pname;
          }
          else {
-            struct gl_debug_state *debug = _mesa_get_debug_state(ctx);
-            *params = debug ? (void *) debug->Callback : NULL;
+            struct gl_debug_state *debug = _mesa_lock_debug_state(ctx);
+            if (debug) {
+               *params = debug->Callback;
+               _mesa_unlock_debug_state(ctx);
+            }
+            else {
+               *params = NULL;
+            }
          }
          break;
       case GL_DEBUG_CALLBACK_USER_PARAM_ARB:
@@ -266,8 +272,14 @@ _mesa_GetPointerv( GLenum pname, GLvoid **params )
             goto invalid_pname;
          }
          else {
-            struct gl_debug_state *debug = _mesa_get_debug_state(ctx);
-            *params = debug ? (void *) debug->CallbackData : NULL;
+            struct gl_debug_state *debug = _mesa_lock_debug_state(ctx);
+            if (debug) {
+               *params = (void *) debug->CallbackData;
+               _mesa_unlock_debug_state(ctx);
+            }
+            else {
+               *params = NULL;
+            }
          }
          break;
       default:
diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h
index 6694383..f94e7f6 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -4215,6 +4215,7 @@ struct gl_context
    GLuint ErrorDebugCount;
 
    /* GL_ARB_debug_output/GL_KHR_debug */
+   mtx_t DebugMutex;
    struct gl_debug_state *Debug;
 
    GLenum RenderMode;        /**< either GL_RENDER, GL_SELECT, GL_FEEDBACK */
diff --git a/src/mesa/state_tracker/st_manager.c b/src/mesa/state_tracker/st_manager.c
index 314d342..fcc8d9e 100644
--- a/src/mesa/state_tracker/st_manager.c
+++ b/src/mesa/state_tracker/st_manager.c
@@ -663,13 +663,14 @@ st_api_create_context(struct st_api *stapi, struct st_manager *smapi,
    }
 
    if (attribs->flags & ST_CONTEXT_FLAG_DEBUG){
-      struct gl_debug_state *debug = _mesa_get_debug_state(st->ctx);
+      struct gl_debug_state *debug = _mesa_lock_debug_state(st->ctx);
       if (!debug) {
          *error = ST_CONTEXT_ERROR_NO_MEMORY;
          return NULL;
       }
       st->ctx->Const.ContextFlags |= GL_CONTEXT_FLAG_DEBUG_BIT;
       debug->DebugOutput = GL_TRUE;
+      _mesa_unlock_debug_state(st->ctx)
    }
 
    if (attribs->flags & ST_CONTEXT_FLAG_FORWARD_COMPATIBLE)
-- 
1.8.5.3



More information about the mesa-dev mailing list