[Mesa-dev] [PATCHv3 10/16] mesa: protect the debug state with a mutex

Chia-I Wu olvaffe at gmail.com
Tue Aug 19 23:40:31 PDT 2014


We are about to change mesa to spawn threads for deferred glCompileShader and
glLinkProgram, and we need to make sure those threads can send compiler
warnings/errors to the debug output safely.

Signed-off-by: Chia-I Wu <olv at lunarg.com>
Reviewed-by: Brian Paul <brianp at vmware.com>
Reviewed-by: Ian Romanick <ian.d.romanick at intel.com>
---
 src/mesa/main/errors.c | 172 +++++++++++++++++++++++++++++++++++--------------
 src/mesa/main/mtypes.h |   1 +
 2 files changed, 126 insertions(+), 47 deletions(-)

diff --git a/src/mesa/main/errors.c b/src/mesa/main/errors.c
index 6b55a1d..218b4ee 100644
--- a/src/mesa/main/errors.c
+++ b/src/mesa/main/errors.c
@@ -677,22 +677,41 @@ 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.  When NULL is returned, the
+ * debug state is not locked.
  */
 static 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");
+         GET_CURRENT_CONTEXT(cur);
+         mtx_unlock(&ctx->DebugMutex);
+
+         /*
+          * This function may be called from other threads.  When that is the
+          * case, we cannot record this OOM error.
+          */
+         if (ctx == cur)
+            _mesa_error(ctx, GL_OUT_OF_MEMORY, "allocating debug state");
+
+         return NULL;
       }
    }
 
    return ctx->Debug;
 }
 
+static void
+_mesa_unlock_debug_state(struct gl_context *ctx)
+{
+   mtx_unlock(&ctx->DebugMutex);
+}
+
 /**
  * Set the integer debug state specified by \p pname.  This can be called from
  * _mesa_set_enable for example.
@@ -700,7 +719,7 @@ _mesa_get_debug_state(struct gl_context *ctx)
 bool
 _mesa_set_debug_state_int(struct gl_context *ctx, GLenum pname, GLint val)
 {
-   struct gl_debug_state *debug = _mesa_get_debug_state(ctx);
+   struct gl_debug_state *debug = _mesa_lock_debug_state(ctx);
 
    if (!debug)
       return false;
@@ -717,6 +736,8 @@ _mesa_set_debug_state_int(struct gl_context *ctx, GLenum pname, GLint val)
       break;
    }
 
+   _mesa_unlock_debug_state(ctx);
+
    return true;
 }
 
@@ -730,9 +751,12 @@ _mesa_get_debug_state_int(struct gl_context *ctx, GLenum pname)
    struct gl_debug_state *debug;
    GLint val;
 
+   mtx_lock(&ctx->DebugMutex);
    debug = ctx->Debug;
-   if (!debug)
+   if (!debug) {
+      mtx_unlock(&ctx->DebugMutex);
       return 0;
+   }
 
    switch (pname) {
    case GL_DEBUG_OUTPUT:
@@ -757,6 +781,8 @@ _mesa_get_debug_state_int(struct gl_context *ctx, GLenum pname)
       break;
    }
 
+   mtx_unlock(&ctx->DebugMutex);
+
    return val;
 }
 
@@ -770,9 +796,12 @@ _mesa_get_debug_state_ptr(struct gl_context *ctx, GLenum pname)
    struct gl_debug_state *debug;
    void *val;
 
+   mtx_lock(&ctx->DebugMutex);
    debug = ctx->Debug;
-   if (!debug)
+   if (!debug) {
+      mtx_unlock(&ctx->DebugMutex);
       return NULL;
+   }
 
    switch (pname) {
    case GL_DEBUG_CALLBACK_FUNCTION_ARB:
@@ -787,9 +816,49 @@ _mesa_get_debug_state_ptr(struct gl_context *ctx, GLenum pname)
       break;
    }
 
+   mtx_unlock(&ctx->DebugMutex);
+
    return val;
 }
 
+/**
+ * Insert a debug message.  The mutex is assumed to be locked, and will be
+ * unlocked by this call.
+ */
+static void
+log_msg_locked_and_unlock(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_enabled(debug, source, type, id, severity)) {
+      _mesa_unlock_debug_state(ctx);
+      return;
+   }
+
+   if (ctx->Debug->Callback) {
+      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;
+
+      /*
+       * 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.
+       */
+      _mesa_unlock_debug_state(ctx);
+      callback(gl_source, gl_type, id, gl_severity, len, buf, data);
+   }
+   else {
+      debug_log_message(ctx->Debug, source, type, id, severity, len, buf);
+      _mesa_unlock_debug_state(ctx);
+   }
+}
 
 /**
  * Log a client or driver debug message.
@@ -799,24 +868,12 @@ 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_enabled(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);
-      return;
-   }
-
-   debug_log_message(debug, source, type, id, severity, len, buf);
+   log_msg_locked_and_unlock(ctx, source, type, id, severity, len, buf);
 }
 
 
@@ -957,7 +1014,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;
 
@@ -992,6 +1049,8 @@ _mesa_GetDebugMessageLog(GLuint count, GLsizei logSize, GLenum *sources,
       debug_delete_messages(debug, 1);
    }
 
+   _mesa_unlock_debug_state(ctx);
+
    return ret;
 }
 
@@ -1028,7 +1087,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;
 
@@ -1040,6 +1099,8 @@ _mesa_DebugMessageControl(GLenum gl_source, GLenum gl_type,
    else {
       debug_set_message_enable_all(debug, source, type, severity, enabled);
    }
+
+   _mesa_unlock_debug_state(ctx);
 }
 
 
@@ -1047,10 +1108,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);
    }
 }
 
@@ -1060,18 +1122,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_message *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:
@@ -1087,10 +1141,15 @@ _mesa_PushDebugGroup(GLenum source, GLuint id, GLsizei length,
    if (!validate_length(ctx, callerstr, length))
       return; /* GL_INVALID_VALUE */
 
-   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;
+   }
 
    /* pop reuses the message details from push so we store this */
    emptySlot = debug_get_group_message(debug);
@@ -1102,6 +1161,12 @@ _mesa_PushDebugGroup(GLenum source, GLuint id, GLsizei length,
                        length, message);
 
    debug_push_group(debug);
+
+   log_msg_locked_and_unlock(ctx,
+         gl_enum_to_debug_source(source),
+         MESA_DEBUG_TYPE_PUSH_GROUP, id,
+         MESA_DEBUG_SEVERITY_NOTIFICATION, length,
+         message);
 }
 
 
@@ -1109,35 +1174,43 @@ 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_message *gdmessage;
+   struct gl_debug_state *debug;
+   struct gl_debug_message *gdmessage, msg;
 
+   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;
    }
 
    debug_pop_group(debug);
 
+   /* make a shallow copy */
    gdmessage = debug_get_group_message(debug);
-   log_msg(ctx, gdmessage->source,
-           gl_enum_to_debug_type(GL_DEBUG_TYPE_POP_GROUP),
-           gdmessage->id,
-           gl_enum_to_debug_severity(GL_DEBUG_SEVERITY_NOTIFICATION),
-           gdmessage->length, gdmessage->message);
-
-   debug_message_clear(gdmessage);
+   msg = *gdmessage;
+   gdmessage->message = NULL;
+   gdmessage->length = 0;
+
+   log_msg_locked_and_unlock(ctx,
+         msg.source,
+         gl_enum_to_debug_type(GL_DEBUG_TYPE_POP_GROUP),
+         msg.id,
+         gl_enum_to_debug_severity(GL_DEBUG_SEVERITY_NOTIFICATION),
+         msg.length, msg.message);
+
+   debug_message_clear(&msg);
 }
 
 
 void
 _mesa_init_errors(struct gl_context *ctx)
 {
-   /* no-op */
+   mtx_init(&ctx->DebugMutex, mtx_plain);
 }
 
 
@@ -1149,6 +1222,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);
 }
 
 
@@ -1363,6 +1438,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_enabled(ctx->Debug,
                                         MESA_DEBUG_SOURCE_API,
@@ -1373,6 +1450,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/mtypes.h b/src/mesa/main/mtypes.h
index 8e5dad5..f2c7013 100644
--- a/src/mesa/main/mtypes.h
+++ b/src/mesa/main/mtypes.h
@@ -4235,6 +4235,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 */
-- 
2.0.0.rc2



More information about the mesa-dev mailing list