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

Commit 23748668 authored by Mathias Agopian's avatar Mathias Agopian
Browse files

fix a deadlock when removing a DisplayEventConnection 

the deadlock would happen when the pipe became invalid and SF
trying to remove the connection from its list.

we know make sure to process events without holding a lock.

Change-Id: I39927ed8824fc7811e16db3c7608a2ebc72d9642
parent 2892cb04

services/surfaceflinger/EventThread.cpp

+43 −21
Original line number Diff line number Diff line
@@ -60,9 +60,20 @@ status_t EventThread::unregisterDisplayEventConnection( 
    return NO_ERROR;

}



status_t EventThread::removeDisplayEventConnection(

        const wp<DisplayEventConnection>& connection) {

    Mutex::Autolock _l(mLock);

    mDisplayEventConnections.remove(connection);

    return NO_ERROR;

}



bool EventThread::threadLoop() {



    nsecs_t timestamp;

    DisplayEventReceiver::Event vsync;

    SortedVector<wp<DisplayEventConnection> > displayEventConnections;



    { // scope for the lock

        Mutex::Autolock _l(mLock);

        do {

            // wait for listeners
@@ -81,15 +92,19 @@ bool EventThread::threadLoop() { 


        // dispatch vsync events to listeners...

        mDeliveredEvents++;

    const size_t count = mDisplayEventConnections.size();



    DisplayEventReceiver::Event vsync;

        vsync.header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC;

        vsync.header.timestamp = timestamp;

        vsync.vsync.count = mDeliveredEvents;



        // make a copy of our connection list, so we can

        // dispatch events without holding mLock

        displayEventConnections = mDisplayEventConnections;

    }



    const size_t count = displayEventConnections.size();

    for (size_t i=0 ; i<count ; i++) {

        sp<DisplayEventConnection> conn(mDisplayEventConnections.itemAt(i).promote());

        sp<DisplayEventConnection> conn(displayEventConnections.itemAt(i).promote());

        // make sure the connection didn't die

        if (conn != NULL) {

            status_t err = conn->postEvent(vsync);
@@ -103,11 +118,18 @@ bool EventThread::threadLoop() { 
                // handle any other error on the pipe as fatal. the only

                // reasonable thing to do is to clean-up this connection.

                // The most common error we'll get here is -EPIPE.

                mDisplayEventConnections.remove(conn);

                removeDisplayEventConnection(displayEventConnections.itemAt(i));

            }

        } else {

            // somehow the connection is dead, but we still have it in our list

            // just clean the list.

            removeDisplayEventConnection(displayEventConnections.itemAt(i));

        }

    }



    // clear all our references without holding mLock

    displayEventConnections.clear();



    return true;

}

services/surfaceflinger/EventThread.h

+3 −0
Original line number Diff line number Diff line
@@ -58,6 +58,9 @@ private: 
    virtual status_t    readyToRun();

    virtual void        onFirstRef();



    status_t removeDisplayEventConnection(

            const wp<DisplayEventConnection>& connection);



    // constants

    sp<SurfaceFlinger> mFlinger;

    const DisplayHardware& mHw;