Ensure memory ordering around libagl and EGL refcount operations

The android_atomic_inc/android_atomic_dec functions don't impose
sufficient memory ordering. Using them for object refcounting could
allow an object to be destroyed prior to writes by a different thread
being visible.

Bug: 28820690
Change-Id: Ie018091035174255a22ebc52852528cdaec2d648

diff --git a/opengl/libagl/BufferObjectManager.h b/opengl/libagl/BufferObjectManager.h
index 6487faa..fcdae5b 100644 (file)
--- a/opengl/libagl/BufferObjectManager.h
+++ b/opengl/libagl/BufferObjectManager.h
@@ -18,11 +18,11 @@
 #ifndef ANDROID_OPENGLES_BUFFER_OBJECT_MANAGER_H
 #define ANDROID_OPENGLES_BUFFER_OBJECT_MANAGER_H
 
+#include <atomic>
 #include <stdint.h>
 #include <stddef.h>
 #include <sys/types.h>
 
-#include <utils/Atomic.h>
 #include <utils/RefBase.h>
 #include <utils/KeyedVector.h>
 #include <utils/Errors.h>
@@ -64,16 +64,17 @@ public:
     void                deleteBuffers(GLsizei n, const GLuint* buffers);
 
 private:
-    mutable volatile int32_t            mCount;
+    mutable std::atomic_size_t          mCount;
     mutable Mutex                       mLock;
     KeyedVector<GLuint, gl::buffer_t*>  mBuffers;
 };
 
 void EGLBufferObjectManager::incStrong(const void* /*id*/) const {
-    android_atomic_inc(&mCount);
+    mCount.fetch_add(1, std::memory_order_relaxed);
 }
 void EGLBufferObjectManager::decStrong(const void* /*id*/) const {
-    if (android_atomic_dec(&mCount) == 1) {
+    if (mCount.fetch_sub(1, std::memory_order_release) == 0) {
+        std::atomic_thread_fence(std::memory_order_acquire);
         delete this;
     }
 }

diff --git a/opengl/libagl/egl.cpp b/opengl/libagl/egl.cpp
index 7560d8f..92139e9 100644 (file)
--- a/opengl/libagl/egl.cpp
+++ b/opengl/libagl/egl.cpp
@@ -16,6 +16,7 @@
 */
 
 #include <assert.h>
+#include <atomic>
 #include <errno.h>
 #include <stdlib.h>
 #include <stdio.h>
@@ -27,7 +28,6 @@
 #include <sys/mman.h>
 
 #include <cutils/log.h>
-#include <cutils/atomic.h>
 
 #include <utils/threads.h>
 #include <ui/ANativeObjectBase.h>
@@ -107,8 +107,8 @@ struct egl_display_t
         return ((uintptr_t(dpy)-1U) >= NUM_DISPLAYS) ? EGL_FALSE : EGL_TRUE;
     }
 
-    NativeDisplayType   type;
-    volatile int32_t    initialized;
+    NativeDisplayType  type;
+    std::atomic_size_t initialized;
 };
 
 static egl_display_t gDisplays[NUM_DISPLAYS];
@@ -1429,7 +1429,7 @@ EGLBoolean eglInitialize(EGLDisplay dpy, EGLint *major, EGLint *minor)
     EGLBoolean res = EGL_TRUE;
     egl_display_t& d = egl_display_t::get_display(dpy);
 
-    if (android_atomic_inc(&d.initialized) == 0) {
+    if (d.initialized.fetch_add(1, std::memory_order_acquire) == 0) {
         // initialize stuff here if needed
         //pthread_mutex_lock(&gInitMutex);
         //pthread_mutex_unlock(&gInitMutex);
@@ -1449,7 +1449,8 @@ EGLBoolean eglTerminate(EGLDisplay dpy)
 
     EGLBoolean res = EGL_TRUE;
     egl_display_t& d = egl_display_t::get_display(dpy);
-    if (android_atomic_dec(&d.initialized) == 1) {
+    if (d.initialized.fetch_sub(1, std::memory_order_release) == 1) {
+        std::atomic_thread_fence(std::memory_order_acquire);
         // TODO: destroy all resources (surfaces, contexts, etc...)
         //pthread_mutex_lock(&gInitMutex);
         //pthread_mutex_unlock(&gInitMutex);

diff --git a/opengl/libs/EGL/egl_object.h b/opengl/libs/EGL/egl_object.h
index 673b7da..8f3b9cb 100644 (file)
--- a/opengl/libs/EGL/egl_object.h
+++ b/opengl/libs/EGL/egl_object.h
@@ -17,7 +17,7 @@
 #ifndef ANDROID_EGL_OBJECT_H
 #define ANDROID_EGL_OBJECT_H
 
-
+#include <atomic>
 #include <ctype.h>
 #include <stdint.h>
 #include <stdlib.h>
@@ -41,7 +41,7 @@ class egl_display_t;
 
 class egl_object_t {
     egl_display_t *display;
-    mutable volatile int32_t count;
+    mutable std::atomic_size_t count;
 
 protected:
     virtual ~egl_object_t();
@@ -51,8 +51,8 @@ public:
     egl_object_t(egl_display_t* display);
     void destroy();
 
-    inline int32_t incRef() { return android_atomic_inc(&count); }
-    inline int32_t decRef() { return android_atomic_dec(&count); }
+    inline void incRef() { count.fetch_add(1, std::memory_order_relaxed); }
+    inline size_t decRef() { return count.fetch_sub(1, std::memory_order_acq_rel); }
     inline egl_display_t* getDisplay() const { return display; }
 
 private: