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

Commit a0fef1c8 authored by Jesse Hall's avatar Jesse Hall
Browse files

Fix deadlock when cleaning objects in eglTerminate 

When eglTerminate() is called with a window surface still exists, a
deadlock would occur since egl_display_t::terminate() holds a lock
while destroying the window surface, which calls
onWindowSurfaceDestroyed() which attempts to take the same lock.

This change refactors the hibernation code and data into a separate
object with its own lock, separate from the egl_display_t lock. This
avoids the deadlock and better encapsulates the hibernation logic.

The change also fixes a bug discovered incidentally while debugging:
hibernating after calling eglTerminate() succeeds, but will cause
awakens from subsequent eglInitialize() to fail. We will no longer
hibernate a terminated display.

Change-Id: If55e5bb603d4f8953babc439ffc8d8a60af103d9
parent 3aecbb07

opengl/libs/EGL/egl_display.cpp

+36 −22
Original line number Diff line number Diff line
@@ -70,8 +70,7 @@ extern void setGLHooksThreadSpecific(gl_hooks_t const *value); 
egl_display_t egl_display_t::sDisplay[NUM_DISPLAYS];



egl_display_t::egl_display_t() :

    magic('_dpy'), finishOnSwap(false), traceGpuCompletion(false), refs(0),

    mWakeCount(0), mHibernating(false), mAttemptHibernation(false) {

    magic('_dpy'), finishOnSwap(false), traceGpuCompletion(false), refs(0) {

}



egl_display_t::~egl_display_t() {
@@ -253,6 +252,9 @@ EGLBoolean egl_display_t::initialize(EGLint *major, EGLint *minor) { 
        *major = VERSION_MAJOR;

    if (minor != NULL)

        *minor = VERSION_MINOR;



    mHibernation.setDisplayValid(true);



    return EGL_TRUE;

}
@@ -282,6 +284,8 @@ EGLBoolean egl_display_t::terminate() { 
        res = EGL_TRUE;

    }



    mHibernation.setDisplayValid(false);



    // Mark all objects remaining in the list as terminated, unless

    // there are no reference to them, it which case, we're free to

    // delete them.
@@ -351,8 +355,7 @@ EGLBoolean egl_display_t::makeCurrent(egl_context_t* c, egl_context_t* cur_c, 
            if (result == EGL_TRUE) {

                c->onMakeCurrent(draw, read);

                if (!cur_c) {

                    mWakeCount++;

                    mAttemptHibernation = false;

                    mHibernation.incWakeCount(HibernationMachine::STRONG);

                }

            }

        } else {
@@ -360,8 +363,7 @@ EGLBoolean egl_display_t::makeCurrent(egl_context_t* c, egl_context_t* cur_c, 
                    disp.dpy, impl_draw, impl_read, impl_ctx);

            if (result == EGL_TRUE) {

                cur_c->onLooseCurrent();

                mWakeCount--;

                mAttemptHibernation = true;

                mHibernation.decWakeCount(HibernationMachine::STRONG);

            }

        }

    }
@@ -378,14 +380,26 @@ EGLBoolean egl_display_t::makeCurrent(egl_context_t* c, egl_context_t* cur_c, 
    return result;

}



bool egl_display_t::enter() {

    Mutex::Autolock _l(lock);

// ----------------------------------------------------------------------------



bool egl_display_t::HibernationMachine::incWakeCount(WakeRefStrength strength) {

    Mutex::Autolock _l(mLock);

    ALOGE_IF(mWakeCount < 0 || mWakeCount == INT32_MAX,

             "Invalid WakeCount (%d) on enter\n", mWakeCount);



    mWakeCount++;

    if (strength == STRONG)

        mAttemptHibernation = false;



    if (CC_UNLIKELY(mHibernating)) {

        ALOGV("Awakening\n");

        egl_connection_t* const cnx = &gEGLImpl;



        // These conditions should be guaranteed before entering hibernation;

        // we don't want to get into a state where we can't wake up.

        ALOGD_IF(!mDpyValid || !cnx->egl.eglAwakenProcessIMG,

                 "Invalid hibernation state, unable to awaken\n");



        if (!cnx->egl.eglAwakenProcessIMG()) {

            ALOGE("Failed to awaken EGL implementation\n");

            return false;
@@ -395,13 +409,20 @@ bool egl_display_t::enter() { 
    return true;

}



void egl_display_t::leave() {

    Mutex::Autolock _l(lock);

void egl_display_t::HibernationMachine::decWakeCount(WakeRefStrength strength) {

    Mutex::Autolock _l(mLock);

    ALOGE_IF(mWakeCount <= 0, "Invalid WakeCount (%d) on leave\n", mWakeCount);

    if (--mWakeCount == 0 && CC_UNLIKELY(mAttemptHibernation)) {



    mWakeCount--;

    if (strength == STRONG)

        mAttemptHibernation = true;



    if (mWakeCount == 0 && CC_UNLIKELY(mAttemptHibernation)) {

        egl_connection_t* const cnx = &gEGLImpl;

        mAttemptHibernation = false;

        if (cnx->egl.eglHibernateProcessIMG && cnx->egl.eglAwakenProcessIMG) {

        if (mDpyValid &&

                cnx->egl.eglHibernateProcessIMG &&

                cnx->egl.eglAwakenProcessIMG) {

            ALOGV("Hibernating\n");

            if (!cnx->egl.eglHibernateProcessIMG()) {

                ALOGE("Failed to hibernate EGL implementation\n");
@@ -412,16 +433,9 @@ void egl_display_t::leave() { 
    }

}



void egl_display_t::onWindowSurfaceCreated() {

    Mutex::Autolock _l(lock);

    mWakeCount++;

    mAttemptHibernation = false;

}



void egl_display_t::onWindowSurfaceDestroyed() {

    Mutex::Autolock _l(lock);

    mWakeCount--;

    mAttemptHibernation = true;

void egl_display_t::HibernationMachine::setDisplayValid(bool valid) {

    Mutex::Autolock _l(mLock);

    mDpyValid = valid;

}



// ----------------------------------------------------------------------------

opengl/libs/EGL/egl_display.h

+43 −7
Original line number Diff line number Diff line
@@ -77,8 +77,12 @@ public: 
    // proper hibernate/wakeup sequencing. If a surface destruction triggers

    // hibernation, hibernation will be delayed at least until the calling

    // thread's egl_display_ptr is destroyed.

    void onWindowSurfaceCreated();

    void onWindowSurfaceDestroyed();

    void onWindowSurfaceCreated() {

        mHibernation.incWakeCount(HibernationMachine::STRONG);

    }

    void onWindowSurfaceDestroyed() {

        mHibernation.decWakeCount(HibernationMachine::STRONG);

    }



    static egl_display_t* get(EGLDisplay dpy);

    static EGLDisplay getFromNativeDisplay(EGLNativeDisplayType disp);
@@ -123,8 +127,8 @@ public: 


private:

    friend class egl_display_ptr;

    bool enter();

    void leave();

    bool enter() { return mHibernation.incWakeCount(HibernationMachine::WEAK); }

    void leave() { return mHibernation.decWakeCount(HibernationMachine::WEAK); }



            uint32_t                    refs;

    mutable Mutex                       lock;
@@ -133,9 +137,41 @@ private: 
            String8 mVersionString;

            String8 mClientApiString;

            String8 mExtensionString;



    // HibernationMachine uses its own internal mutex to protect its own data.

    // The owning egl_display_t's lock may be but is not required to be held

    // when calling HibernationMachine methods. As a result, nothing in this

    // class may call back up to egl_display_t directly or indirectly.

    class HibernationMachine {

    public:

        // STRONG refs cancel (inc) or initiate (dec) a hibernation attempt

        // the next time the wakecount reaches zero. WEAK refs don't affect

        // whether a hibernation attempt will be made. Use STRONG refs only

        // for infrequent/heavy changes that are likely to indicate the

        // EGLDisplay is entering or leaving a long-term idle state.

        enum WakeRefStrength {

            WEAK   = 0,

            STRONG = 1,

        };



        HibernationMachine(): mWakeCount(0), mHibernating(false),

                mAttemptHibernation(false), mDpyValid(false)

        {}

        ~HibernationMachine() {}



        bool incWakeCount(WakeRefStrength strenth);

        void decWakeCount(WakeRefStrength strenth);



        void setDisplayValid(bool valid);



    private:

        Mutex   mLock;

        int32_t mWakeCount;

        bool    mHibernating;

        bool    mAttemptHibernation;

        bool    mDpyValid;

    };

    HibernationMachine mHibernation;

};



// ----------------------------------------------------------------------------