Merge branch 'tests-fix-leaks' into 'master'

Fix memory leaks in unit tests

See merge request glvnd/libglvnd!237

tests/dummy/EGL_dummy.c

@@ -65,11 +65,18 @@ typedef struct DummyThreadStateRec {
     EGLint lastError;
     EGLContext currentContext;
     EGLLabelKHR label;
+
+    struct glvnd_list entry;
 } DummyThreadState;
 
 static const __EGLapiExports *apiExports = NULL;
+
+static glvnd_mutex_t threadStateLock = GLVND_MUTEX_INITIALIZER;
 static glvnd_key_t threadStateKey;
-static struct glvnd_list displayList;
+static struct glvnd_list threadStateList = { &threadStateList, &threadStateList };
+
+static struct glvnd_list displayList = { &displayList, &displayList };
+static glvnd_mutex_t displayListLock = GLVND_MUTEX_INITIALIZER;
 static EGLint failNextMakeCurrentError = EGL_NONE;
 
 static EGLDEBUGPROCKHR debugCallbackFunc = NULL;
@@ -86,6 +93,9 @@ static DummyThreadState *GetThreadState(void)
             abort();
         }
         thr->lastError = EGL_SUCCESS;
+        __glvndPthreadFuncs.mutex_lock(&threadStateLock);
+        glvnd_list_append(&thr->entry, &threadStateList);
+        __glvndPthreadFuncs.mutex_unlock(&threadStateLock);
         __glvndPthreadFuncs.setspecific(threadStateKey, thr);
     }
     return thr;
@@ -117,11 +127,15 @@ static void SetLastError(const char *command, EGLLabelKHR label, EGLint error)
 static DummyEGLDisplay *LookupEGLDisplay(EGLDisplay dpy)
 {
     DummyEGLDisplay *disp = NULL;
+
+    __glvndPthreadFuncs.mutex_lock(&displayListLock);
     glvnd_list_for_each_entry(disp, &displayList, entry) {
         if (dpy == (EGLDisplay) disp) {
+            __glvndPthreadFuncs.mutex_unlock(&displayListLock);
             return disp;
         }
     }
+    __glvndPthreadFuncs.mutex_unlock(&displayListLock);
     // Libglvnd should never pass an invalid EGLDisplay handle to a vendor
     // library.
     printf("Invalid EGLDisplay %p\n", dpy);
@@ -199,8 +213,10 @@ static EGLDisplay dummyGetPlatformDisplay(EGLenum platform, void *native_display
         return EGL_NO_DISPLAY;
     }
 
+    __glvndPthreadFuncs.mutex_lock(&displayListLock);
     glvnd_list_for_each_entry(disp, &displayList, entry) {
         if (disp->platform == platform && disp->native_display == native_display) {
+            __glvndPthreadFuncs.mutex_unlock(&displayListLock);
             return disp;
         }
     }
@@ -210,6 +226,7 @@ static EGLDisplay dummyGetPlatformDisplay(EGLenum platform, void *native_display
     disp->platform = platform;
     disp->native_display = native_display;
     glvnd_list_append(&disp->entry, &displayList);
+    __glvndPthreadFuncs.mutex_unlock(&displayListLock);
     return disp;
 }
 
@@ -804,3 +821,29 @@ __egl_Main(uint32_t version, const __EGLapiExports *exports,
     return EGL_TRUE;
 }
 
+#if defined(USE_ATTRIBUTE_CONSTRUCTOR)
+void __attribute__ ((destructor)) __eglDummyFini(void)
+#else
+void _fini(void)
+#endif
+{
+    if (apiExports == NULL) {
+        // We were never initialized, so there's nothing to clean up.
+        return;
+    }
+
+    while (!glvnd_list_is_empty(&displayList)) {
+        DummyEGLDisplay *disp = glvnd_list_first_entry(
+                &displayList, DummyEGLDisplay, entry);
+        glvnd_list_del(&disp->entry);
+        free(disp);
+    }
+
+    __glvndPthreadFuncs.key_delete(threadStateKey);
+    while (!glvnd_list_is_empty(&threadStateList)) {
+        DummyThreadState *thr = glvnd_list_first_entry(
+                &threadStateList, DummyThreadState, entry);
+        glvnd_list_del(&thr->entry);
+        free(thr);
+    }
+}

tests/dummy/GLX_dummy.c

@@ -44,9 +44,7 @@ static const __GLXapiExports *apiExports = NULL;
  * Dummy context structure.
  */
 typedef struct __GLXcontextRec {
-    GLint beginHit;
+    GLContextCounts counts;
-    GLint vertex3fvHit;
-    GLint endHit;
 } __GLXcontext;
 
 static const int FBCONFIGS_PER_SCREEN = 10;
@@ -62,8 +60,8 @@ static void dummy_glXExampleExtensionFunction(Display *dpy, int screen, int *ret
 static void dispatch_glXExampleExtensionFunction(Display *dpy, int screen, int *retval);
 static void dummy_glXExampleExtensionFunction2(Display *dpy, int screen, int *retval);
 static void dispatch_glXExampleExtensionFunction2(Display *dpy, int screen, int *retval);
-static void dummy_glXMakeCurrentTestResults(GLint req, GLboolean *saw, void **ret);
+static void dummy_glXMakeCurrentTestResults(GLboolean *saw, GLContextCounts *counts);
-static void dispatch_glXMakeCurrentTestResults(GLint req, GLboolean *saw, void **ret);
+static void dispatch_glXMakeCurrentTestResults(GLboolean *saw, GLContextCounts *counts);
 
 enum
 {
@@ -154,10 +152,7 @@ static void          dummy_glXCopyContext           (Display *dpy,
 static GLXContext CommonCreateContext(Display *dpy, int screen)
 {
     if (screen >= 0) {
-        __GLXcontext *context = malloc(sizeof(*context));
+        __GLXcontext *context = calloc(1, sizeof(*context));
-        context->beginHit = 0;
-        context->vertex3fvHit = 0;
-        context->endHit = 0;
         return context;
     } else {
         return NULL;
@@ -502,7 +497,7 @@ static void dummy_glBegin (void)
     GLXContext ctx = apiExports->getCurrentContext();
     assert(ctx);
 
-    ctx->beginHit++;
+    ctx->counts.beginCount++;
 }
 
 static void dummy_glVertex3fv(GLfloat *v)
@@ -510,7 +505,7 @@ static void dummy_glVertex3fv(GLfloat *v)
     GLXContext ctx = apiExports->getCurrentContext();
     assert(ctx);
 
-    ctx->vertex3fvHit++;
+    ctx->counts.vertex3fvCount++;
 }
 
 static void dummy_glEnd (void)
@@ -518,33 +513,19 @@ static void dummy_glEnd (void)
     GLXContext ctx = apiExports->getCurrentContext();
     assert(ctx);
 
-    ctx->endHit++;
+    ctx->counts.endCount++;
 }
 
-static void dummy_glXMakeCurrentTestResults(GLint req, GLboolean *saw, void **ret)
+static void dummy_glXMakeCurrentTestResults(GLboolean *saw, GLContextCounts *counts)
 {
     GLXContext ctx = apiExports->getCurrentContext();
     assert(ctx);
 
     *saw = GL_TRUE;
-    switch (req) {
+    *counts = ctx->counts;
-    case GL_MC_FUNCTION_COUNTS:
-        {
-            GLint *data = (GLint *)malloc(3 * sizeof(GLint));
-            data[0] = ctx->beginHit;
-            data[1] = ctx->vertex3fvHit;
-            data[2] = ctx->endHit;
-            *ret = (void *)data;
-        }
-        break;
-    case GL_MC_LAST_REQ:
-    default:
-        *ret = NULL;
-        break;
-    }
 }
 
-static void dispatch_glXMakeCurrentTestResults(GLint req, GLboolean *saw, void **ret)
+static void dispatch_glXMakeCurrentTestResults(GLboolean *saw, GLContextCounts *counts)
 {
     __GLXvendorInfo *dynDispatch;
     PFNGLXMAKECURRENTTESTRESULTSPROC func;
@@ -558,7 +539,7 @@ static void dispatch_glXMakeCurrentTestResults(GLint req, GLboolean *saw, void *
     func = (PFNGLXMAKECURRENTTESTRESULTSPROC)
         apiExports->fetchDispatchEntry(dynDispatch, index);
     if (func) {
-        func(req, saw, ret);
+        func(saw, counts);
     }
 }
 

tests/dummy/GLX_dummy.h

@@ -35,20 +35,12 @@
  * GLX_makecurrent vendor library used in the testglxmakecurrent test.
  */
 
-enum {
+typedef struct
-
+{
-    /*
+    GLint beginCount;
-     * Returns an array of 3 GLint values containing, respectively,
+    GLint vertex3fvCount;
-     * the number of times glBegin(), glVertex3fv(), and glEnd() were called
+    GLint endCount;
-     * by this thread.
+} GLContextCounts;
-     */
-    GL_MC_FUNCTION_COUNTS,
-
-    /*
-     * Last request. Always returns NULL.
-     */
-    GL_MC_LAST_REQ
-} GLmakeCurrentTestRequest;
 
 /**
  * This is an attribute to query using glXQueryContext to test dispatching by
@@ -66,29 +58,22 @@ enum {
  */
 typedef void (* PFNGLXEXAMPLEEXTENSIONFUNCTION) (Display *dpy, int screen, int *retval);
 
-/*
+/**
  * glXMakeCurrentTestResults(): perform queries on vendor library state.
  *
  * This explicitly is designed to not return anything, in case a bug causes the
  * API library to dispatch this to a no-op stub. If this function returned a
  * value and dispatched to a no-op, the return value would be bogus and hard to
  * debug.  To detect this issue, clients should initialize *saw to GL_FALSE
- * before passing it to this function. Similarly, *ret should be initialized to
+ * before passing it to this function.
- * NULL prior to passing it to this function.
  *
- * \param [in] req The request to perform. Must be a valid
- * GLmakeCurrentTestRequest enum
  * \param [out] saw Expected to point to a GLboolean initialied to GL_FALSE.
  * *saw is set to GL_TRUE if we dispatched to the vendor function.
- * \param [out] ret Expected to point to a (void*) initialized to NULL. *ret is
+ * \param [out] counts Should point to a GLContextCounts struct. This returns the
- * set to NULL if there was an error, or a pointer to request-specific data
+ *      number of GL calls that the vendor library has seen with the current
- * otherwise. The pointer may be passed into free(3).
+ *      context.
  */
-typedef void (*PFNGLXMAKECURRENTTESTRESULTSPROC)(
+typedef void (*PFNGLXMAKECURRENTTESTRESULTSPROC) (GLboolean *saw, GLContextCounts *counts);
-    GLint req,
-    GLboolean *saw,
-    void **ret
-);
 
 /**
  * glXCreateContextVendorDUMMY(): Dummy extension function to create a context.

tests/testglxgetprocaddress.c

@@ -91,6 +91,9 @@ int main(int argc, char **argv)
         return 1;
     }
 
+    XCloseDisplay(dpy);
+    printf("Success\n");
+
     // Success!
     return 0;
 }

tests/testglxmakecurrent.c

@@ -109,15 +109,11 @@ void *MakeCurrentThread(void *arg)
     PFNGLXMAKECURRENTTESTRESULTSPROC pMakeCurrentTestResults = NULL;
     GLXContext ctx = NULL;
     const GLfloat v[] = { 0, 0, 0 };
-    struct {
+    GLboolean saw;
-        GLint req;
+    GLContextCounts contextCounts = {};
-        GLboolean saw;
-        void *ret;
-    } makeCurrentTestResultsParams;
     GLint BeginCount = 0;
     GLint EndCount = 0;
     GLint Vertex3fvCount = 0;
-    GLint *vendorCounts;
     int i;
     intptr_t ret = GL_FALSE;
     const TestOptions *t = (const TestOptions *)arg;
@@ -170,32 +166,21 @@ void *MakeCurrentThread(void *arg)
         glVertex3fv(v); Vertex3fvCount++;
         glEnd(); EndCount++;
 
-        // Make a call to glXMakeCurrentTestResults() to get the function counts.
+        // Make a call to glXMakeCurrentTestResults() to get the function contextCounts.
-        makeCurrentTestResultsParams.req = GL_MC_FUNCTION_COUNTS;
+        saw = GL_FALSE;
-        makeCurrentTestResultsParams.saw = GL_FALSE;
+        memset(&contextCounts, 0, sizeof(contextCounts));
-        makeCurrentTestResultsParams.ret = NULL;
+        pMakeCurrentTestResults(&saw, &contextCounts);
 
-        pMakeCurrentTestResults(makeCurrentTestResultsParams.req,
+        if (!saw) {
-                                &makeCurrentTestResultsParams.saw,
-                                &makeCurrentTestResultsParams.ret);
-
-        if (!makeCurrentTestResultsParams.saw) {
             printError("Failed to dispatch glXMakeCurrentTestResults()!\n");
             goto fail;
         }
 
-        if (!makeCurrentTestResultsParams.ret) {
+        // Verify we have the right function contextCounts
-            printError("Internal glXMakeCurrentTestResults() error!\n");
+        if ((contextCounts.beginCount != BeginCount) ||
-            goto fail;
+            (contextCounts.vertex3fvCount != Vertex3fvCount) ||
-        }
+            (contextCounts.endCount != EndCount)) {
-
+            printError("Mismatch of reported function call contextCounts "
-        // Verify we have the right function counts
-        vendorCounts = (GLint *)makeCurrentTestResultsParams.ret;
-
-        if ((vendorCounts[0] != BeginCount) ||
-            (vendorCounts[1] != Vertex3fvCount) ||
-            (vendorCounts[2] != EndCount)) {
-            printError("Mismatch of reported function call counts "
                        "between the application and vendor library!\n");
             goto fail;
         }
@@ -213,15 +198,9 @@ void *MakeCurrentThread(void *arg)
 
         // Similarly the call to the dynamic function glXMakeCurrentTestResults()
         // should be a no-op.
-        makeCurrentTestResultsParams.req = GL_MC_FUNCTION_COUNTS;
+        saw = GL_FALSE;
-        makeCurrentTestResultsParams.saw = GL_FALSE;
+        pMakeCurrentTestResults(&saw, &contextCounts);
-        makeCurrentTestResultsParams.ret = NULL;
+        if (saw) {
-
-        pMakeCurrentTestResults(makeCurrentTestResultsParams.req,
-                                &makeCurrentTestResultsParams.saw,
-                                &makeCurrentTestResultsParams.ret);
-
-        if (makeCurrentTestResultsParams.saw) {
             printError("Dynamic function glXMakeCurrentTestResults() dispatched "
                        "to vendor library even though no context was current!\n");
             goto fail;
@@ -233,10 +212,13 @@ void *MakeCurrentThread(void *arg)
     ret = GL_TRUE;
 
 fail:
-    if (ctx) {
+    if (dpy != NULL) {
-        glXDestroyContext(dpy, ctx);
+        if (ctx) {
+            glXDestroyContext(dpy, ctx);
+        }
+        testUtilsDestroyWindow(dpy, &wi);
+        XCloseDisplay(dpy);
     }
-    testUtilsDestroyWindow(dpy, &wi);
 
     return (void *)ret;
 }
@@ -289,6 +271,7 @@ int main(int argc, char **argv)
                 all_ret = 1;
             }
         }
+        free(threads);
     }
     return all_ret;
 }