summaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
authorSteinar H. Gunderson <[email protected]>2016-02-02 01:16:51 +0100
committerMarek Olšák <[email protected]>2016-02-05 17:18:17 +0100
commitfeb53912f8d8c29594a9fdff914d78bb36d6d56b (patch)
tree4ca9c61e7609a67a65789ad85202376b768f2df0 /src
parent156e81f305b19f62b4d6ea98bf44a951e89dc947 (diff)
mesa: Fix locking of GLsync objects.
GLsync objects had a race condition when used from multiple threads (which is the main point of the extension, really); it could be validated as a sync object at the beginning of the function, and then deleted by another thread before use, causing crashes. Fix this by changing all casts from GLsync to struct gl_sync_object to a new function _mesa_get_and_ref_sync() that validates and increases the refcount. In a similar vein, validation itself uses _mesa_set_search(), which requires synchronization -- it was called without a mutex held, causing spurious error returns and other issues. Since _mesa_get_and_ref_sync() now takes the shared context mutex, this problem is also resolved. Fixes bug #92757, found while developing Nageru, my live video mixer (due for release at FOSDEM 2016). v2: Marek: silence warnings, fix declaration after code Signed-off-by: Steinar H. Gunderson <[email protected]> Cc: "11.0 11.1" <[email protected]> Signed-off-by: Marek Olšák <[email protected]>
Diffstat (limited to 'src')
-rw-r--r--src/mesa/main/objectlabel.c13
-rw-r--r--src/mesa/main/shared.c2
-rw-r--r--src/mesa/main/syncobj.c89
-rw-r--r--src/mesa/main/syncobj.h11
4 files changed, 66 insertions, 49 deletions
diff --git a/src/mesa/main/objectlabel.c b/src/mesa/main/objectlabel.c
index 41f370ce485..b622d6a2979 100644
--- a/src/mesa/main/objectlabel.c
+++ b/src/mesa/main/objectlabel.c
@@ -288,16 +288,18 @@ void GLAPIENTRY
_mesa_ObjectPtrLabel(const void *ptr, GLsizei length, const GLchar *label)
{
GET_CURRENT_CONTEXT(ctx);
- struct gl_sync_object *const syncObj = (struct gl_sync_object *) ptr;
+ struct gl_sync_object *syncObj;
const char *callerstr;
char **labelPtr;
+ syncObj = _mesa_get_and_ref_sync(ctx, (void*)ptr, true);
+
if (_mesa_is_desktop_gl(ctx))
callerstr = "glObjectPtrLabel";
else
callerstr = "glObjectPtrLabelKHR";
- if (!_mesa_validate_sync(ctx, syncObj)) {
+ if (!syncObj) {
_mesa_error(ctx, GL_INVALID_VALUE, "%s (not a valid sync object)",
callerstr);
return;
@@ -306,6 +308,7 @@ _mesa_ObjectPtrLabel(const void *ptr, GLsizei length, const GLchar *label)
labelPtr = &syncObj->Label;
set_label(ctx, labelPtr, label, length, callerstr);
+ _mesa_unref_sync_object(ctx, syncObj, 1);
}
void GLAPIENTRY
@@ -313,7 +316,7 @@ _mesa_GetObjectPtrLabel(const void *ptr, GLsizei bufSize, GLsizei *length,
GLchar *label)
{
GET_CURRENT_CONTEXT(ctx);
- struct gl_sync_object *const syncObj = (struct gl_sync_object *) ptr;
+ struct gl_sync_object *syncObj;
const char *callerstr;
char **labelPtr;
@@ -328,7 +331,8 @@ _mesa_GetObjectPtrLabel(const void *ptr, GLsizei bufSize, GLsizei *length,
return;
}
- if (!_mesa_validate_sync(ctx, syncObj)) {
+ syncObj = _mesa_get_and_ref_sync(ctx, (void*)ptr, true);
+ if (!syncObj) {
_mesa_error(ctx, GL_INVALID_VALUE, "%s (not a valid sync object)",
callerstr);
return;
@@ -337,4 +341,5 @@ _mesa_GetObjectPtrLabel(const void *ptr, GLsizei bufSize, GLsizei *length,
labelPtr = &syncObj->Label;
copy_label(*labelPtr, label, length, bufSize);
+ _mesa_unref_sync_object(ctx, syncObj, 1);
}
diff --git a/src/mesa/main/shared.c b/src/mesa/main/shared.c
index c37b31d1753..b9f7bb65fb6 100644
--- a/src/mesa/main/shared.c
+++ b/src/mesa/main/shared.c
@@ -338,7 +338,7 @@ free_shared_state(struct gl_context *ctx, struct gl_shared_state *shared)
struct set_entry *entry;
set_foreach(shared->SyncObjects, entry) {
- _mesa_unref_sync_object(ctx, (struct gl_sync_object *) entry->key);
+ _mesa_unref_sync_object(ctx, (struct gl_sync_object *) entry->key, 1);
}
}
_mesa_set_destroy(shared->SyncObjects, NULL);
diff --git a/src/mesa/main/syncobj.c b/src/mesa/main/syncobj.c
index c1b2d3bed54..be758dd1241 100644
--- a/src/mesa/main/syncobj.c
+++ b/src/mesa/main/syncobj.c
@@ -167,34 +167,42 @@ _mesa_free_sync_data(struct gl_context *ctx)
* - not in sync objects hash table
* - type is GL_SYNC_FENCE
* - not marked as deleted
+ *
+ * Returns the internal gl_sync_object pointer if the sync object is valid
+ * or NULL if it isn't.
+ *
+ * If "incRefCount" is true, the reference count is incremented, which is
+ * normally what you want; otherwise, a glDeleteSync from another thread
+ * could delete the sync object while you are still working on it.
*/
-bool
-_mesa_validate_sync(struct gl_context *ctx,
- const struct gl_sync_object *syncObj)
+struct gl_sync_object *
+_mesa_get_and_ref_sync(struct gl_context *ctx, GLsync sync, bool incRefCount)
{
- return (syncObj != NULL)
+ struct gl_sync_object *syncObj = (struct gl_sync_object *) sync;
+ mtx_lock(&ctx->Shared->Mutex);
+ if (syncObj != NULL
&& _mesa_set_search(ctx->Shared->SyncObjects, syncObj) != NULL
&& (syncObj->Type == GL_SYNC_FENCE)
- && !syncObj->DeletePending;
-}
-
-
-void
-_mesa_ref_sync_object(struct gl_context *ctx, struct gl_sync_object *syncObj)
-{
- mtx_lock(&ctx->Shared->Mutex);
- syncObj->RefCount++;
+ && !syncObj->DeletePending) {
+ if (incRefCount) {
+ syncObj->RefCount++;
+ }
+ } else {
+ syncObj = NULL;
+ }
mtx_unlock(&ctx->Shared->Mutex);
+ return syncObj;
}
void
-_mesa_unref_sync_object(struct gl_context *ctx, struct gl_sync_object *syncObj)
+_mesa_unref_sync_object(struct gl_context *ctx, struct gl_sync_object *syncObj,
+ int amount)
{
struct set_entry *entry;
mtx_lock(&ctx->Shared->Mutex);
- syncObj->RefCount--;
+ syncObj->RefCount -= amount;
if (syncObj->RefCount == 0) {
entry = _mesa_set_search(ctx->Shared->SyncObjects, syncObj);
assert (entry != NULL);
@@ -212,10 +220,9 @@ GLboolean GLAPIENTRY
_mesa_IsSync(GLsync sync)
{
GET_CURRENT_CONTEXT(ctx);
- struct gl_sync_object *const syncObj = (struct gl_sync_object *) sync;
ASSERT_OUTSIDE_BEGIN_END_WITH_RETVAL(ctx, GL_FALSE);
- return _mesa_validate_sync(ctx, syncObj) ? GL_TRUE : GL_FALSE;
+ return _mesa_get_and_ref_sync(ctx, sync, false) ? GL_TRUE : GL_FALSE;
}
@@ -223,7 +230,7 @@ void GLAPIENTRY
_mesa_DeleteSync(GLsync sync)
{
GET_CURRENT_CONTEXT(ctx);
- struct gl_sync_object *const syncObj = (struct gl_sync_object *) sync;
+ struct gl_sync_object *syncObj;
/* From the GL_ARB_sync spec:
*
@@ -235,16 +242,19 @@ _mesa_DeleteSync(GLsync sync)
return;
}
- if (!_mesa_validate_sync(ctx, syncObj)) {
+ syncObj = _mesa_get_and_ref_sync(ctx, sync, true);
+ if (!syncObj) {
_mesa_error(ctx, GL_INVALID_VALUE, "glDeleteSync (not a valid sync object)");
return;
}
/* If there are no client-waits or server-waits pending on this sync, delete
- * the underlying object.
+ * the underlying object. Note that we double-unref the object, as
+ * _mesa_get_and_ref_sync above took an extra refcount to make sure the pointer
+ * is valid for us to manipulate.
*/
syncObj->DeletePending = GL_TRUE;
- _mesa_unref_sync_object(ctx, syncObj);
+ _mesa_unref_sync_object(ctx, syncObj, 2);
}
@@ -299,21 +309,20 @@ GLenum GLAPIENTRY
_mesa_ClientWaitSync(GLsync sync, GLbitfield flags, GLuint64 timeout)
{
GET_CURRENT_CONTEXT(ctx);
- struct gl_sync_object *const syncObj = (struct gl_sync_object *) sync;
+ struct gl_sync_object *syncObj;
GLenum ret;
ASSERT_OUTSIDE_BEGIN_END_WITH_RETVAL(ctx, GL_WAIT_FAILED);
- if (!_mesa_validate_sync(ctx, syncObj)) {
- _mesa_error(ctx, GL_INVALID_VALUE, "glClientWaitSync (not a valid sync object)");
- return GL_WAIT_FAILED;
- }
-
if ((flags & ~GL_SYNC_FLUSH_COMMANDS_BIT) != 0) {
_mesa_error(ctx, GL_INVALID_VALUE, "glClientWaitSync(flags=0x%x)", flags);
return GL_WAIT_FAILED;
}
- _mesa_ref_sync_object(ctx, syncObj);
+ syncObj = _mesa_get_and_ref_sync(ctx, sync, true);
+ if (!syncObj) {
+ _mesa_error(ctx, GL_INVALID_VALUE, "glClientWaitSync (not a valid sync object)");
+ return GL_WAIT_FAILED;
+ }
/* From the GL_ARB_sync spec:
*
@@ -335,7 +344,7 @@ _mesa_ClientWaitSync(GLsync sync, GLbitfield flags, GLuint64 timeout)
}
}
- _mesa_unref_sync_object(ctx, syncObj);
+ _mesa_unref_sync_object(ctx, syncObj, 1);
return ret;
}
@@ -344,12 +353,7 @@ void GLAPIENTRY
_mesa_WaitSync(GLsync sync, GLbitfield flags, GLuint64 timeout)
{
GET_CURRENT_CONTEXT(ctx);
- struct gl_sync_object *const syncObj = (struct gl_sync_object *) sync;
-
- if (!_mesa_validate_sync(ctx, syncObj)) {
- _mesa_error(ctx, GL_INVALID_VALUE, "glWaitSync (not a valid sync object)");
- return;
- }
+ struct gl_sync_object *syncObj;
if (flags != 0) {
_mesa_error(ctx, GL_INVALID_VALUE, "glWaitSync(flags=0x%x)", flags);
@@ -362,7 +366,14 @@ _mesa_WaitSync(GLsync sync, GLbitfield flags, GLuint64 timeout)
return;
}
+ syncObj = _mesa_get_and_ref_sync(ctx, sync, true);
+ if (!syncObj) {
+ _mesa_error(ctx, GL_INVALID_VALUE, "glWaitSync (not a valid sync object)");
+ return;
+ }
+
ctx->Driver.ServerWaitSync(ctx, syncObj, flags, timeout);
+ _mesa_unref_sync_object(ctx, syncObj, 1);
}
@@ -371,11 +382,12 @@ _mesa_GetSynciv(GLsync sync, GLenum pname, GLsizei bufSize, GLsizei *length,
GLint *values)
{
GET_CURRENT_CONTEXT(ctx);
- struct gl_sync_object *const syncObj = (struct gl_sync_object *) sync;
+ struct gl_sync_object *syncObj;
GLsizei size = 0;
GLint v[1];
- if (!_mesa_validate_sync(ctx, syncObj)) {
+ syncObj = _mesa_get_and_ref_sync(ctx, sync, true);
+ if (!syncObj) {
_mesa_error(ctx, GL_INVALID_VALUE, "glGetSynciv (not a valid sync object)");
return;
}
@@ -409,6 +421,7 @@ _mesa_GetSynciv(GLsync sync, GLenum pname, GLsizei bufSize, GLsizei *length,
default:
_mesa_error(ctx, GL_INVALID_ENUM, "glGetSynciv(pname=0x%x)\n", pname);
+ _mesa_unref_sync_object(ctx, syncObj, 1);
return;
}
@@ -421,4 +434,6 @@ _mesa_GetSynciv(GLsync sync, GLenum pname, GLsizei bufSize, GLsizei *length,
if (length != NULL) {
*length = size;
}
+
+ _mesa_unref_sync_object(ctx, syncObj, 1);
}
diff --git a/src/mesa/main/syncobj.h b/src/mesa/main/syncobj.h
index 5d510e873a9..ea4a71222c0 100644
--- a/src/mesa/main/syncobj.h
+++ b/src/mesa/main/syncobj.h
@@ -47,15 +47,12 @@ _mesa_init_sync(struct gl_context *);
extern void
_mesa_free_sync_data(struct gl_context *);
-extern void
-_mesa_ref_sync_object(struct gl_context *ctx, struct gl_sync_object *syncObj);
+struct gl_sync_object *
+_mesa_get_and_ref_sync(struct gl_context *ctx, GLsync sync, bool incRefCount);
extern void
-_mesa_unref_sync_object(struct gl_context *ctx, struct gl_sync_object *syncObj);
-
-extern bool
-_mesa_validate_sync(struct gl_context *ctx,
- const struct gl_sync_object *syncObj);
+_mesa_unref_sync_object(struct gl_context *ctx, struct gl_sync_object *syncObj,
+ int amount);
extern GLboolean GLAPIENTRY
_mesa_IsSync(GLsync sync);