egl: Introduce rwlock to protect eglTerminate()

eglTerminate() must be serialized against all other EGL calls.  But in
most cases, other EGL calls do not need to be serialized against each
other.  Which fits rather well with a rwlock.

One would be tempted to simply replace the existing BDL with a rwlock,
but several portability and debuggability limitations of the rwlock
implementation prevent that, as described in the TerminateLock comment
block.

Signed-off-by: Rob Clark <robdclark@chromium.org>
Acked-by: Eric Engestrom <eric@igalia.com>
Reviewed-by: Adam Jackson <ajax@redhat.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/18050>

src/egl/drivers/dri2/egl_dri2.c

@@ -656,9 +656,9 @@ dri2_validate_egl_image(void *image, void *data)
    _EGLDisplay *disp = data;
    _EGLImage *img;
 
-   simple_mtx_lock(&disp->Mutex);
+   egl_lock(disp);
    img = _eglLookupImage(image, disp);
-   simple_mtx_unlock(&disp->Mutex);
+   egl_unlock(disp);
 
    if (img == NULL) {
       _eglError(EGL_BAD_PARAMETER, "dri2_validate_egl_image");

src/egl/drivers/wgl/egl_wgl.c

@@ -215,9 +215,9 @@ wgl_validate_egl_image(struct st_manager *smapi, void *image)
    _EGLDisplay *disp = wgl_dpy->parent;
    _EGLImage *img;
 
-   simple_mtx_lock(&disp->Mutex);
+   egl_lock(disp);
    img = _eglLookupImage(image, disp);
-   simple_mtx_unlock(&disp->Mutex);
+   egl_unlock(disp);
 
    if (img == NULL) {
       _eglError(EGL_BAD_PARAMETER, "wgl_validate_egl_image");

src/egl/main/eglapi.c

@@ -250,7 +250,7 @@ _eglLockDisplay(EGLDisplay dpy)
 {
    _EGLDisplay *disp = _eglLookupDisplay(dpy);
    if (disp)
-      simple_mtx_lock(&disp->Mutex);
+      egl_lock(disp);
    return disp;
 }
 
@@ -261,7 +261,7 @@ _eglLockDisplay(EGLDisplay dpy)
 static inline void
 _eglUnlockDisplay(_EGLDisplay *disp)
 {
-   simple_mtx_unlock(&disp->Mutex);
+   egl_unlock(disp);
 }
 
 static void
@@ -689,13 +689,16 @@ eglInitialize(EGLDisplay dpy, EGLint *major, EGLint *minor)
 EGLBoolean EGLAPIENTRY
 eglTerminate(EGLDisplay dpy)
 {
-   _EGLDisplay *disp = _eglLockDisplay(dpy);
+   _EGLDisplay *disp = _eglLookupDisplay(dpy);
 
    _EGL_FUNC_START(disp, EGL_OBJECT_DISPLAY_KHR, NULL);
 
    if (!disp)
       RETURN_EGL_ERROR(NULL, EGL_BAD_DISPLAY, EGL_FALSE);
 
+   u_rwlock_wrlock(&disp->TerminateLock);
+   simple_mtx_lock(&disp->Mutex);
+
    if (disp->Initialized) {
       disp->Driver->Terminate(disp);
       /* do not reset disp->Driver */
@@ -707,7 +710,10 @@ eglTerminate(EGLDisplay dpy)
       disp->BlobCacheGet = NULL;
    }
 
-   RETURN_EGL_SUCCESS(disp, EGL_TRUE);
+   simple_mtx_unlock(&disp->Mutex);
+   u_rwlock_wrunlock(&disp->TerminateLock);
+
+   RETURN_EGL_SUCCESS(NULL, EGL_TRUE);
 }
 
 
@@ -1523,7 +1529,7 @@ _eglWaitClientCommon(void)
       RETURN_EGL_SUCCESS(NULL, EGL_TRUE);
 
    disp = ctx->Resource.Display;
-   simple_mtx_lock(&disp->Mutex);
+   egl_lock(disp);
 
    /* let bad current context imply bad current surface */
    if (_eglGetContextHandle(ctx) == EGL_NO_CONTEXT ||
@@ -1566,7 +1572,7 @@ eglWaitNative(EGLint engine)
    _EGL_FUNC_START(NULL, EGL_OBJECT_THREAD_KHR, NULL);
 
    disp = ctx->Resource.Display;
-   simple_mtx_lock(&disp->Mutex);
+   egl_lock(disp);
 
    /* let bad current context imply bad current surface */
    if (_eglGetContextHandle(ctx) == EGL_NO_CONTEXT ||
@@ -1725,9 +1731,9 @@ eglReleaseThread(void)
    if (ctx) {
       _EGLDisplay *disp = ctx->Resource.Display;
 
-      simple_mtx_lock(&disp->Mutex);
+      egl_lock(disp);
       (void) disp->Driver->MakeCurrent(disp, NULL, NULL, NULL);
-      simple_mtx_unlock(&disp->Mutex);
+      egl_unlock(disp);
    }
 
    _eglDestroyCurrentThread();

src/egl/main/egldisplay.c

@@ -279,6 +279,7 @@ _eglFindDisplay(_EGLPlatformType plat, void *plat_dpy,
       goto out;
 
    simple_mtx_init(&disp->Mutex, mtx_plain);
+   u_rwlock_init(&disp->TerminateLock);
    disp->Platform = plat;
    disp->PlatformDisplay = plat_dpy;
    num_attribs = _eglNumAttribs(attrib_list);

src/egl/main/egldisplay.h

@@ -32,6 +32,7 @@
 #define EGLDISPLAY_INCLUDED
 
 #include "util/simple_mtx.h"
+#include "util/rwlock.h"
 
 #include "egltypedefs.h"
 #include "egldefines.h"
@@ -159,8 +160,34 @@ struct _egl_display
    /* used to link displays */
    _EGLDisplay *Next;
 
+   /**
+    * The big-display-lock (BDL) which protects our internal state.  EGL
+    * drivers should use their own locking, as needed, to protect their
+    * own state, rather than relying on this.
+    */
    simple_mtx_t Mutex;
 
+   /**
+    * The spec appears to allow eglTerminate() to race with more or less
+    * any other egl call.  To allow for this, while relaxing the BDL to
+    * allow other egl calls to happen in parallel, a rwlock is used.  All
+    * points where the BDL lock is acquired also acquire TerminateLock
+    * for reading, while eglTerminate() itself acquires the TerminateLock
+    * for writing.
+    *
+    * Note, we could conceivably just replace the BDL with a single
+    * rwlock.  But there are a couple shortcomings of u_rwlock:
+    *
+    *   1) The WIN32 implementation does not allow promoting a read-
+    *      lock to write-lock, nor recursive locking, whereas the
+    *      pthread based implementation does.  Because of this, it
+    *      would be difficult to keep the eglapi layer portable if
+    *      we depended on any less-than-trivial rwlock usage.
+    *
+    *   2) We'd lose simple_mtx_assert_locked().
+    */
+   struct u_rwlock TerminateLock;
+
    _EGLPlatformType Platform; /**< The type of the platform display */
    void *PlatformDisplay;     /**< A pointer to the platform display */
 
@@ -198,6 +225,19 @@ struct _egl_display
    EGLGetBlobFuncANDROID BlobCacheGet;
 };
 
+static inline void
+egl_lock(_EGLDisplay *disp)
+{
+   u_rwlock_rdlock(&disp->TerminateLock);
+   simple_mtx_lock(&disp->Mutex);
+}
+
+static inline void
+egl_unlock(_EGLDisplay *disp)
+{
+   simple_mtx_unlock(&disp->Mutex);
+   u_rwlock_rdunlock(&disp->TerminateLock);
+}
 
 extern _EGLPlatformType
 _eglGetNativePlatform(void *nativeDisplay);