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.

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 <[email protected]>
  */
 
@@ -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 <sync> was signaled, even if the value of <timeout> 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);