More thread safety in ES2 & EGL APIs

Tweak ContextPtr to handle move construction/assignment correctly.
This was overlooked when I made the last minute change to use explicit lock/unlock.

Tweak LockGuard to allow construction from ptr to MutexLock, and behave
correctly if mutex == nullptr. This is required because the locking
machinery for EGL is /outside/ the error generation for bad EGLDisplay.

Add 'Big EGL Lock' around most of the EGL API

Bug: b/112184433
Change-Id: Ic4cd5fa9e6a16ae81cbb2b46e1cb881a850749c5
Reviewed-on: https://swiftshader-review.googlesource.com/20628
Reviewed-by: Nicolas Capens <nicolascapens@google.com>
Tested-by: Chris Forbes <chrisforbes@google.com>

diff --git a/src/Common/MutexLock.hpp b/src/Common/MutexLock.hpp
index 3a071c9..65e9fa4 100644 (file)
--- a/src/Common/MutexLock.hpp
+++ b/src/Common/MutexLock.hpp
@@ -177,18 +177,23 @@ namespace sw
 class LockGuard
 {
 public:
-       explicit LockGuard(sw::MutexLock &mutex) : mutex(mutex)
+       explicit LockGuard(sw::MutexLock &mutex) : mutex(&mutex)
        {
                mutex.lock();
        }
 
+       explicit LockGuard(sw::MutexLock *mutex) : mutex(mutex)
+       {
+               if (mutex) mutex->lock();
+       }
+
        ~LockGuard()
        {
-               mutex.unlock();
+               if (mutex) mutex->unlock();
        }
 
 protected:
-       sw::MutexLock &mutex;
+       sw::MutexLock *mutex;
 };
 
 #endif   // sw_MutexLock_hpp

diff --git a/src/OpenGL/libEGL/Display.cpp b/src/OpenGL/libEGL/Display.cpp
index 8bbed9e..8e8b9d3 100644 (file)
--- a/src/OpenGL/libEGL/Display.cpp
+++ b/src/OpenGL/libEGL/Display.cpp
@@ -598,7 +598,6 @@ EGLContext Display::createContext(EGLConfig configHandle, const egl::Context *sh
 EGLSyncKHR Display::createSync(Context *context)
 {
        FenceSync *fenceSync = new egl::FenceSync(context);
-       LockGuard lock(mSyncSetMutex);
        mSyncSet.insert(fenceSync);
        return fenceSync;
 }
@@ -635,7 +634,6 @@ void Display::destroyContext(egl::Context *context)
 void Display::destroySync(FenceSync *sync)
 {
        {
-               LockGuard lock(mSyncSetMutex);
                mSyncSet.erase(sync);
        }
        delete sync;
@@ -717,7 +715,6 @@ bool Display::hasExistingWindowSurface(EGLNativeWindowType window)
 
 bool Display::isValidSync(FenceSync *sync)
 {
-       LockGuard lock(mSyncSetMutex);
        return mSyncSet.find(sync) != mSyncSet.end();
 }
 

diff --git a/src/OpenGL/libEGL/Display.h b/src/OpenGL/libEGL/Display.h
index ddef14a..53ebdfb 100644 (file)
--- a/src/OpenGL/libEGL/Display.h
+++ b/src/OpenGL/libEGL/Display.h
@@ -86,6 +86,8 @@ namespace egl
                bool destroySharedImage(EGLImageKHR);
                virtual Image *getSharedImage(EGLImageKHR name) = 0;
 
+               sw::MutexLock *getLock() { return &mApiMutex; }
+
        private:
                sw::Format getDisplayFormat() const;
 
@@ -104,10 +106,10 @@ namespace egl
                ContextSet mContextSet;
 
                typedef std::set<FenceSync*> SyncSet;
-               sw::MutexLock mSyncSetMutex;
                SyncSet mSyncSet;
 
                gl::NameSpace<Image> mSharedImageNameSpace;
+               sw::MutexLock mApiMutex;
        };
 }
 

diff --git a/src/OpenGL/libEGL/main.cpp b/src/OpenGL/libEGL/main.cpp
index c6c5630..0ef957c 100644 (file)
--- a/src/OpenGL/libEGL/main.cpp
+++ b/src/OpenGL/libEGL/main.cpp
@@ -19,6 +19,7 @@
 #include "libEGL.hpp"
 #include "Context.hpp"
 #include "Surface.hpp"
+#include "Display.h"
 
 #include "resource.h"
 #include "Common/Thread.hpp"
@@ -330,6 +331,13 @@ void error(EGLint errorCode)
                }
        }
 }
+
+sw::MutexLock *getDisplayLock(EGLDisplay dpy)
+{
+       auto display = Display::get(dpy);
+       if (!display) return nullptr;
+       return display->getLock();
+}
 }
 
 namespace egl
@@ -399,56 +407,67 @@ EGLAPI EGLDisplay EGLAPIENTRY eglGetDisplay(EGLNativeDisplayType display_id)
 
 EGLAPI EGLBoolean EGLAPIENTRY eglInitialize(EGLDisplay dpy, EGLint *major, EGLint *minor)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::Initialize(dpy, major, minor);
 }
 
 EGLAPI EGLBoolean EGLAPIENTRY eglTerminate(EGLDisplay dpy)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::Terminate(dpy);
 }
 
 EGLAPI const char *EGLAPIENTRY eglQueryString(EGLDisplay dpy, EGLint name)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::QueryString(dpy, name);
 }
 
 EGLAPI EGLBoolean EGLAPIENTRY eglGetConfigs(EGLDisplay dpy, EGLConfig *configs, EGLint config_size, EGLint *num_config)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::GetConfigs(dpy, configs, config_size, num_config);
 }
 
 EGLAPI EGLBoolean EGLAPIENTRY eglChooseConfig(EGLDisplay dpy, const EGLint *attrib_list, EGLConfig *configs, EGLint config_size, EGLint *num_config)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::ChooseConfig(dpy, attrib_list, configs, config_size, num_config);
 }
 
 EGLAPI EGLBoolean EGLAPIENTRY eglGetConfigAttrib(EGLDisplay dpy, EGLConfig config, EGLint attribute, EGLint *value)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::GetConfigAttrib(dpy, config, attribute, value);
 }
 
 EGLAPI EGLSurface EGLAPIENTRY eglCreateWindowSurface(EGLDisplay dpy, EGLConfig config, EGLNativeWindowType window, const EGLint *attrib_list)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::CreateWindowSurface(dpy, config, window, attrib_list);
 }
 
 EGLAPI EGLSurface EGLAPIENTRY eglCreatePbufferSurface(EGLDisplay dpy, EGLConfig config, const EGLint *attrib_list)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::CreatePbufferSurface(dpy, config, attrib_list);
 }
 
 EGLAPI EGLSurface EGLAPIENTRY eglCreatePixmapSurface(EGLDisplay dpy, EGLConfig config, EGLNativePixmapType pixmap, const EGLint *attrib_list)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::CreatePixmapSurface(dpy, config, pixmap, attrib_list);
 }
 
 EGLAPI EGLBoolean EGLAPIENTRY eglDestroySurface(EGLDisplay dpy, EGLSurface surface)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::DestroySurface(dpy, surface);
 }
 
 EGLAPI EGLBoolean EGLAPIENTRY eglQuerySurface(EGLDisplay dpy, EGLSurface surface, EGLint attribute, EGLint *value)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::QuerySurface(dpy, surface, attribute, value);
 }
 
@@ -474,41 +493,49 @@ EGLAPI EGLBoolean EGLAPIENTRY eglReleaseThread(void)
 
 EGLAPI EGLSurface EGLAPIENTRY eglCreatePbufferFromClientBuffer(EGLDisplay dpy, EGLenum buftype, EGLClientBuffer buffer, EGLConfig config, const EGLint *attrib_list)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::CreatePbufferFromClientBuffer(dpy, buftype, buffer, config, attrib_list);
 }
 
 EGLAPI EGLBoolean EGLAPIENTRY eglSurfaceAttrib(EGLDisplay dpy, EGLSurface surface, EGLint attribute, EGLint value)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::SurfaceAttrib(dpy, surface, attribute, value);
 }
 
 EGLAPI EGLBoolean EGLAPIENTRY eglBindTexImage(EGLDisplay dpy, EGLSurface surface, EGLint buffer)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::BindTexImage(dpy, surface, buffer);
 }
 
 EGLAPI EGLBoolean EGLAPIENTRY eglReleaseTexImage(EGLDisplay dpy, EGLSurface surface, EGLint buffer)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::ReleaseTexImage(dpy, surface, buffer);
 }
 
 EGLAPI EGLBoolean EGLAPIENTRY eglSwapInterval(EGLDisplay dpy, EGLint interval)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::SwapInterval(dpy, interval);
 }
 
 EGLAPI EGLContext EGLAPIENTRY eglCreateContext(EGLDisplay dpy, EGLConfig config, EGLContext share_context, const EGLint *attrib_list)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::CreateContext(dpy, config, share_context, attrib_list);
 }
 
 EGLAPI EGLBoolean EGLAPIENTRY eglDestroyContext(EGLDisplay dpy, EGLContext ctx)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::DestroyContext(dpy, ctx);
 }
 
 EGLAPI EGLBoolean EGLAPIENTRY eglMakeCurrent(EGLDisplay dpy, EGLSurface draw, EGLSurface read, EGLContext ctx)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::MakeCurrent(dpy, draw, read, ctx);
 }
 
@@ -529,6 +556,7 @@ EGLAPI EGLDisplay EGLAPIENTRY eglGetCurrentDisplay(void)
 
 EGLAPI EGLBoolean EGLAPIENTRY eglQueryContext(EGLDisplay dpy, EGLContext ctx, EGLint attribute, EGLint *value)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::QueryContext(dpy, ctx, attribute, value);
 }
 
@@ -544,31 +572,37 @@ EGLAPI EGLBoolean EGLAPIENTRY eglWaitNative(EGLint engine)
 
 EGLAPI EGLBoolean EGLAPIENTRY eglSwapBuffers(EGLDisplay dpy, EGLSurface surface)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::SwapBuffers(dpy, surface);
 }
 
 EGLAPI EGLBoolean EGLAPIENTRY eglCopyBuffers(EGLDisplay dpy, EGLSurface surface, EGLNativePixmapType target)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::CopyBuffers(dpy, surface, target);
 }
 
 EGLAPI EGLImageKHR EGLAPIENTRY eglCreateImageKHR(EGLDisplay dpy, EGLContext ctx, EGLenum target, EGLClientBuffer buffer, const EGLint *attrib_list)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::CreateImageKHR(dpy, ctx, target, buffer, attrib_list);
 }
 
 EGLAPI EGLImageKHR EGLAPIENTRY eglCreateImage(EGLDisplay dpy, EGLContext ctx, EGLenum target, EGLClientBuffer buffer, const EGLAttrib *attrib_list)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::CreateImage(dpy, ctx, target, buffer, attrib_list);
 }
 
 EGLAPI EGLBoolean EGLAPIENTRY eglDestroyImageKHR(EGLDisplay dpy, EGLImageKHR image)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::DestroyImageKHR(dpy, image);
 }
 
 EGLAPI EGLBoolean EGLAPIENTRY eglDestroyImage(EGLDisplay dpy, EGLImageKHR image)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::DestroyImageKHR(dpy, image);
 }
 
@@ -584,71 +618,85 @@ EGLAPI EGLDisplay EGLAPIENTRY eglGetPlatformDisplay(EGLenum platform, void *nati
 
 EGLAPI EGLSurface EGLAPIENTRY eglCreatePlatformWindowSurfaceEXT(EGLDisplay dpy, EGLConfig config, void *native_window, const EGLint *attrib_list)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::CreatePlatformWindowSurfaceEXT(dpy, config, native_window, attrib_list);
 }
 
 EGLAPI EGLSurface EGLAPIENTRY eglCreatePlatformWindowSurface(EGLDisplay dpy, EGLConfig config, void *native_window, const EGLAttrib *attrib_list)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::CreatePlatformWindowSurface(dpy, config, native_window, attrib_list);
 }
 
 EGLAPI EGLSurface EGLAPIENTRY eglCreatePlatformPixmapSurfaceEXT(EGLDisplay dpy, EGLConfig config, void *native_pixmap, const EGLint *attrib_list)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::CreatePlatformPixmapSurfaceEXT(dpy, config, native_pixmap, attrib_list);
 }
 
 EGLAPI EGLSurface EGLAPIENTRY eglCreatePlatformPixmapSurface(EGLDisplay dpy, EGLConfig config, void *native_pixmap, const EGLAttrib *attrib_list)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::CreatePlatformPixmapSurface(dpy, config, native_pixmap, attrib_list);
 }
 
 EGLAPI EGLSyncKHR EGLAPIENTRY eglCreateSyncKHR(EGLDisplay dpy, EGLenum type, const EGLint *attrib_list)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::CreateSyncKHR(dpy, type, attrib_list);
 }
 
 EGLAPI EGLSyncKHR EGLAPIENTRY eglCreateSync(EGLDisplay dpy, EGLenum type, const EGLAttrib *attrib_list)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::CreateSync(dpy, type, attrib_list);
 }
 
 EGLAPI EGLBoolean EGLAPIENTRY eglDestroySyncKHR(EGLDisplay dpy, EGLSyncKHR sync)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::DestroySyncKHR(dpy, sync);
 }
 
 EGLAPI EGLBoolean EGLAPIENTRY eglDestroySync(EGLDisplay dpy, EGLSyncKHR sync)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::DestroySyncKHR(dpy, sync);
 }
 
 EGLAPI EGLint EGLAPIENTRY eglClientWaitSyncKHR(EGLDisplay dpy, EGLSyncKHR sync, EGLint flags, EGLTimeKHR timeout)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::ClientWaitSyncKHR(dpy, sync, flags, timeout);
 }
 
 EGLAPI EGLint EGLAPIENTRY eglClientWaitSync(EGLDisplay dpy, EGLSyncKHR sync, EGLint flags, EGLTimeKHR timeout)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::ClientWaitSyncKHR(dpy, sync, flags, timeout);
 }
 
 EGLAPI EGLBoolean EGLAPIENTRY eglGetSyncAttribKHR(EGLDisplay dpy, EGLSyncKHR sync, EGLint attribute, EGLint *value)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::GetSyncAttribKHR(dpy, sync, attribute, value);
 }
 
 EGLAPI EGLBoolean EGLAPIENTRY eglGetSyncAttrib(EGLDisplay dpy, EGLSyncKHR sync, EGLint attribute, EGLAttrib *value)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::GetSyncAttrib(dpy, sync, attribute, value);
 }
 
 EGLAPI EGLint EGLAPIENTRY eglWaitSyncKHR(EGLDisplay dpy, EGLSyncKHR sync, EGLint flags)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::ClientWaitSyncKHR(dpy, sync, flags, EGL_FOREVER_KHR);
 }
 
 EGLAPI EGLBoolean EGLAPIENTRY eglWaitSync(EGLDisplay dpy, EGLSync sync, EGLint flags)
 {
+       LockGuard lock(egl::getDisplayLock(dpy));
        return egl::ClientWaitSyncKHR(dpy, sync, flags, EGL_FOREVER_KHR);
 }
 

diff --git a/src/OpenGL/libGLESv2/Context.h b/src/OpenGL/libGLESv2/Context.h
index 866982b..5e3449f 100644 (file)
--- a/src/OpenGL/libGLESv2/Context.h
+++ b/src/OpenGL/libGLESv2/Context.h
@@ -776,13 +776,18 @@ class ContextPtr {
 public:
        explicit ContextPtr(Context *context) : ptr(context)
        {
-               if (ptr) ptr->getResourceLock()->lock();
-    }
+               if (ptr) { ptr->getResourceLock()->lock(); }
+       }
 
        ~ContextPtr() {
-               if (ptr) ptr->getResourceLock()->unlock();
+               if (ptr) { ptr->getResourceLock()->unlock(); }
        }
 
+       ContextPtr(ContextPtr const &) = delete;
+       ContextPtr & operator=(ContextPtr const &) = delete;
+       ContextPtr(ContextPtr && other) : ptr(other.ptr) { other.ptr = nullptr; }
+       ContextPtr & operator=(ContextPtr && other) { ptr = other.ptr; other.ptr = nullptr; return *this; }
+
        Context *operator ->() { return ptr; }
        operator bool() const { return ptr != nullptr; }