From 16b393d05990b6e917e144f9de87d0103b4c3e6d Mon Sep 17 00:00:00 2001 From: Ian Romanick Date: Mon, 31 Aug 2009 14:57:50 -0700 Subject: [PATCH] ARB sync: Fix delete behavior and context destruction behavior I believe this resolves the outstanding issues WRT sync object deletetion. I have also added a large comment at the top of syncobj.c describing the expected memory management behavior. I'm still a little uncertain about the locking on ctx->Shared. --- src/mesa/main/mtypes.h | 6 +++ src/mesa/main/shared.c | 19 +++++++- src/mesa/main/syncobj.c | 96 +++++++++++++++++++++++++++++++---------- src/mesa/main/syncobj.h | 6 +++ 4 files changed, 103 insertions(+), 24 deletions(-) diff --git a/src/mesa/main/mtypes.h b/src/mesa/main/mtypes.h index 58da5d15eb0..bd3bf28328b 100644 --- a/src/mesa/main/mtypes.h +++ b/src/mesa/main/mtypes.h @@ -40,6 +40,7 @@ #include "main/mfeatures.h" #include "glapi/glapi.h" #include "math/m_matrix.h" /* GLmatrix */ +#include "main/simple_list.h" /* struct simple_node */ /** @@ -1988,6 +1989,7 @@ struct gl_query_state /** Sync object state */ struct gl_sync_object { + struct simple_node link; GLenum Type; /**< GL_SYNC_FENCE */ GLuint Name; /**< Fence name */ GLint RefCount; /**< Reference count */ @@ -2145,6 +2147,10 @@ struct gl_shared_state struct _mesa_HashTable *FrameBuffers; #endif +#if FEATURE_ARB_sync + struct simple_node SyncObjects; +#endif + void *DriverData; /**< Device driver shared state */ }; diff --git a/src/mesa/main/shared.c b/src/mesa/main/shared.c index 93bbccd3c7f..643ad3354e7 100644 --- a/src/mesa/main/shared.c +++ b/src/mesa/main/shared.c @@ -43,7 +43,9 @@ #if FEATURE_ATI_fragment_shader #include "shader/atifragshader.h" #endif - +#if FEATURE_ARB_sync +#include "syncobj.h" +#endif /** * Allocate and initialize a shared context state structure. @@ -127,6 +129,10 @@ _mesa_alloc_shared_state(GLcontext *ctx) shared->RenderBuffers = _mesa_NewHashTable(); #endif +#if FEATURE_ARB_sync + make_empty_list(& shared->SyncObjects); +#endif + return shared; } @@ -336,6 +342,17 @@ _mesa_free_shared_state(GLcontext *ctx, struct gl_shared_state *shared) ctx->Driver.DeleteBuffer(ctx, shared->NullBufferObj); #endif +#if FEATURE_ARB_sync + { + struct simple_node *node; + struct simple_node *temp; + + foreach_s(node, temp, & shared->SyncObjects) { + _mesa_unref_sync_object(ctx, (struct gl_sync_object *) node); + } + } +#endif + /* * Free texture objects (after FBOs since some textures might have * been bound to FBOs). diff --git a/src/mesa/main/syncobj.c b/src/mesa/main/syncobj.c index eeeeb49175f..0471a0ad7f9 100644 --- a/src/mesa/main/syncobj.c +++ b/src/mesa/main/syncobj.c @@ -25,6 +25,33 @@ * \file syncobj.c * Sync object management. * + * Unlike textures and other objects that are shared between contexts, sync + * objects are not bound to the context. As a result, the reference counting + * and delete behavior of sync objects is slightly different. References to + * sync objects are added: + * + * - By \c glFencSynce. This sets the initial reference count to 1. + * - At the start of \c glClientWaitSync. The reference is held for the + * duration of the wait call. + * + * References are removed: + * + * - By \c glDeleteSync. + * - At the end of \c glClientWaitSync. + * + * Additionally, drivers may call \c _mesa_ref_sync_object and + * \c _mesa_unref_sync_object as needed to implement \c ServerWaitSync. + * + * As with shader objects, sync object names become invalid as soon as + * \c glDeleteSync is called. For this reason \c glDeleteSync sets the + * \c DeletePending flag. All functions validate object handles by testing + * this flag. + * + * \note + * Only \c GL_ARB_sync objects are shared between contexts. If support is ever + * added for either \c GL_NV_fence or \c GL_APPLE_fence different semantics + * will need to be implemented. + * * \author Ian Romanick */ @@ -130,31 +157,52 @@ _mesa_free_sync_data(GLcontext *ctx) } -GLboolean -_mesa_IsSync(GLsync sync) +static int +_mesa_validate_sync(struct gl_sync_object *syncObj) { - 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 (syncObj != NULL) + && (syncObj->Type == GL_SYNC_FENCE) + && !syncObj->DeletePending; +} - return ((syncObj != NULL) && (syncObj->Type == GL_SYNC_FENCE)) - ? GL_TRUE : GL_FALSE; +void +_mesa_ref_sync_object(GLcontext *ctx, struct gl_sync_object *syncObj) +{ + _glthread_LOCK_MUTEX(ctx->Shared->Mutex); + syncObj->RefCount++; + _glthread_UNLOCK_MUTEX(ctx->Shared->Mutex); } -static void +void _mesa_unref_sync_object(GLcontext *ctx, struct gl_sync_object *syncObj) { + _glthread_LOCK_MUTEX(ctx->Shared->Mutex); syncObj->RefCount--; if (syncObj->RefCount == 0) { + remove_from_list(& syncObj->link); + _glthread_UNLOCK_MUTEX(ctx->Shared->Mutex); + (*ctx->Driver.DeleteSyncObject)(ctx, syncObj); } else { - syncObj->DeletePending = 1; + _glthread_UNLOCK_MUTEX(ctx->Shared->Mutex); } } +GLboolean +_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(syncObj) ? GL_TRUE : GL_FALSE; +} + + void _mesa_DeleteSync(GLsync sync) { @@ -173,7 +221,7 @@ _mesa_DeleteSync(GLsync sync) return; } - if (syncObj->Type != GL_SYNC_FENCE) { + if (!_mesa_validate_sync(syncObj)) { _mesa_error(ctx, GL_INVALID_OPERATION, "glDeleteSync"); return; } @@ -182,6 +230,7 @@ _mesa_DeleteSync(GLsync sync) /* If there are no client-waits or server-waits pending on this sync, delete * the underlying object. */ + syncObj->DeletePending = GL_TRUE; _mesa_unref_sync_object(ctx, syncObj); } @@ -224,6 +273,10 @@ _mesa_FenceSync(GLenum condition, GLbitfield flags) (*ctx->Driver.FenceSync)(ctx, syncObj, condition, flags); + _glthread_LOCK_MUTEX(ctx->Shared->Mutex); + insert_at_tail(& ctx->Shared->SyncObjects, & syncObj->link); + _glthread_UNLOCK_MUTEX(ctx->Shared->Mutex); + return (GLsync) syncObj; } @@ -240,7 +293,7 @@ _mesa_ClientWaitSync(GLsync sync, GLbitfield flags, GLuint64 timeout) ASSERT_OUTSIDE_BEGIN_END_WITH_RETVAL(ctx, GL_WAIT_FAILED); - if ((syncObj == NULL) || (syncObj->Type != GL_SYNC_FENCE)) { + if (!_mesa_validate_sync(syncObj)) { _mesa_error(ctx, GL_INVALID_OPERATION, "glClientWaitSync"); return GL_WAIT_FAILED; } @@ -251,6 +304,8 @@ _mesa_ClientWaitSync(GLsync sync, GLbitfield flags, GLuint64 timeout) } + _mesa_ref_sync_object(ctx, syncObj); + /* From the GL_ARB_sync spec: * * ClientWaitSync returns one of four status values. A return value of @@ -259,20 +314,15 @@ _mesa_ClientWaitSync(GLsync sync, GLbitfield flags, GLuint64 timeout) * if was signaled, even if the value of is zero. */ (*ctx->Driver.CheckSync)(ctx, syncObj); - if (syncObj->Status) { - return GL_ALREADY_SIGNALED; - } - - - (*ctx->Driver.ClientWaitSync)(ctx, syncObj, flags, timeout); - - ret = syncObj->Status ? GL_CONDITION_SATISFIED : GL_TIMEOUT_EXPIRED; + ret = GL_ALREADY_SIGNALED; + } else { + (*ctx->Driver.ClientWaitSync)(ctx, syncObj, flags, timeout); - if (syncObj->DeletePending && syncObj->Status) { - _mesa_unref_sync_object(ctx, syncObj); + ret = syncObj->Status ? GL_CONDITION_SATISFIED : GL_TIMEOUT_EXPIRED; } + _mesa_unref_sync_object(ctx, syncObj); return ret; } @@ -285,7 +335,7 @@ _mesa_WaitSync(GLsync sync, GLbitfield flags, GLuint64 timeout) ASSERT_OUTSIDE_BEGIN_END(ctx); - if ((syncObj == NULL) || (syncObj->Type != GL_SYNC_FENCE)) { + if (!_mesa_validate_sync(syncObj)) { _mesa_error(ctx, GL_INVALID_OPERATION, "glWaitSync"); return; } @@ -318,7 +368,7 @@ _mesa_GetSynciv(GLsync sync, GLenum pname, GLsizei bufSize, GLsizei *length, ASSERT_OUTSIDE_BEGIN_END(ctx); - if ((syncObj == NULL) || (syncObj->Type != GL_SYNC_FENCE)) { + if (!_mesa_validate_sync(syncObj)) { _mesa_error(ctx, GL_INVALID_OPERATION, "glGetSynciv"); return; } diff --git a/src/mesa/main/syncobj.h b/src/mesa/main/syncobj.h index d2b4d051c9d..fc160af2893 100644 --- a/src/mesa/main/syncobj.h +++ b/src/mesa/main/syncobj.h @@ -42,6 +42,12 @@ _mesa_init_sync(GLcontext *); extern void _mesa_free_sync_data(GLcontext *); +extern void +_mesa_ref_sync_object(GLcontext *ctx, struct gl_sync_object *syncObj); + +extern void +_mesa_unref_sync_object(GLcontext *ctx, struct gl_sync_object *syncObj); + extern GLboolean _mesa_IsSync(GLsync sync); -- 2.30.2