Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 3aa75f95 authored by Jesse Hall's avatar Jesse Hall
Browse files

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
parent b59de7fa

opengl/libagl/BufferObjectManager.h

+5 −4
Original line number Diff line number Diff line
@@ -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;

    }

}

opengl/libagl/egl.cpp

+6 −5
Original line number Diff line number Diff line
@@ -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>
@@ -108,7 +108,7 @@ struct egl_display_t 
    }



    NativeDisplayType  type;

    volatile int32_t    initialized;

    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);

opengl/libs/EGL/egl_object.h

+4 −4
Original line number Diff line number Diff line
@@ -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: