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

Commit 10125f00 authored by Mathias Agopian's avatar Mathias Agopian
Browse files

Fix deadlock in SF.

problem was that we were acquiring a strong reference
on Connection object with a lock held, when those
got out of scope (lock still held) their dtor
could be called if all other refs had dropped,
the dtor would acquire the lock again to
remove the Connection from the main list. boom.

we rearange the code so this doesn't happen.

Bug: 6942208

Change-Id: I0a0ebabce2842d29d60d645b64aac2f26640e59b
parent 2c7eb92b
Loading
Loading
Loading
Loading
+86 −98
Original line number Original line Diff line number Diff line
@@ -36,10 +36,9 @@ namespace android {


EventThread::EventThread(const sp<SurfaceFlinger>& flinger)
EventThread::EventThread(const sp<SurfaceFlinger>& flinger)
    : mFlinger(flinger),
    : mFlinger(flinger),
      mLastVSyncTimestamp(0),
      mVSyncTimestamp(0),
      mVSyncTimestamp(0),
      mUseSoftwareVSync(false),
      mUseSoftwareVSync(false),
      mDeliveredEvents(0),
      mVSyncCount(0),
      mDebugVsyncEnabled(false) {
      mDebugVsyncEnabled(false) {
}
}


@@ -116,34 +115,40 @@ void EventThread::onScreenAcquired() {
void EventThread::onVSyncReceived(int, nsecs_t timestamp) {
void EventThread::onVSyncReceived(int, nsecs_t timestamp) {
    Mutex::Autolock _l(mLock);
    Mutex::Autolock _l(mLock);
    mVSyncTimestamp = timestamp;
    mVSyncTimestamp = timestamp;
    mVSyncCount++;
    mCondition.broadcast();
    mCondition.broadcast();
}
}


bool EventThread::threadLoop() {
bool EventThread::threadLoop() {


    nsecs_t timestamp;
    nsecs_t timestamp;
    size_t vsyncCount;
    size_t activeEvents;
    DisplayEventReceiver::Event vsync;
    DisplayEventReceiver::Event vsync;
    Vector< wp<EventThread::Connection> > displayEventConnections;
    Vector< sp<EventThread::Connection> > activeConnections;


    do {
    do {
        Mutex::Autolock _l(mLock);
        Mutex::Autolock _l(mLock);
        do {
        // latch VSYNC event if any
        // latch VSYNC event if any
        timestamp = mVSyncTimestamp;
        timestamp = mVSyncTimestamp;
        mVSyncTimestamp = 0;
        mVSyncTimestamp = 0;


        // check if we should be waiting for VSYNC events
        // check if we should be waiting for VSYNC events
        activeEvents = 0;
        bool waitForNextVsync = false;
        bool waitForNextVsync = false;
        size_t count = mDisplayEventConnections.size();
        size_t count = mDisplayEventConnections.size();
        for (size_t i=0 ; i<count ; i++) {
        for (size_t i=0 ; i<count ; i++) {
                sp<Connection> connection =
            sp<Connection> connection(mDisplayEventConnections[i].promote());
                        mDisplayEventConnections.itemAt(i).promote();
            if (connection != NULL) {
                if (connection!=0 && connection->count >= 0) {
                activeConnections.add(connection);
                if (connection->count >= 0) {
                    // at least one continuous mode or active one-shot event
                    // at least one continuous mode or active one-shot event
                    waitForNextVsync = true;
                    waitForNextVsync = true;
                    activeEvents++;
                    break;
                    break;
                }
                }
            }
            }
        }


        if (timestamp) {
        if (timestamp) {
            if (!waitForNextVsync) {
            if (!waitForNextVsync) {
@@ -172,54 +177,43 @@ bool EventThread::threadLoop() {
            // is, we just need to make sure to serve the clients
            // is, we just need to make sure to serve the clients
            if (mCondition.waitRelative(mLock, ms2ns(16)) == TIMED_OUT) {
            if (mCondition.waitRelative(mLock, ms2ns(16)) == TIMED_OUT) {
                mVSyncTimestamp = systemTime(SYSTEM_TIME_MONOTONIC);
                mVSyncTimestamp = systemTime(SYSTEM_TIME_MONOTONIC);
                mVSyncCount++;
            }
            }
        } else {
        } else {
            mCondition.wait(mLock);
            mCondition.wait(mLock);
        }
        }
        } while(true);
        vsyncCount = mVSyncCount;

    } while (!activeConnections.size());
        // process vsync event
        mDeliveredEvents++;
        mLastVSyncTimestamp = timestamp;


        // now see if we still need to report this VSYNC event
    if (!activeEvents) {
        const size_t count = mDisplayEventConnections.size();
        // no events to return. start over.
        for (size_t i=0 ; i<count ; i++) {
        // (here we make sure to exit the scope of this function
            bool reportVsync = false;
        //  so that we release our Connection references)
            sp<Connection> connection =
        return true;
                    mDisplayEventConnections.itemAt(i).promote();
            if (connection == 0)
                continue;

            const int32_t count = connection->count;
            if (count >= 1) {
                if (count==1 || (mDeliveredEvents % count) == 0) {
                    // continuous event, and time to report it
                    reportVsync = true;
                }
            } else if (count >= -1) {
                if (count == 0) {
                    // fired this time around
                    reportVsync = true;
                }
                connection->count--;
            }
            if (reportVsync) {
                displayEventConnections.add(connection);
            }
    }
    }
    } while (!displayEventConnections.size());


    // dispatch vsync events to listeners...
    // dispatch vsync events to listeners...
    vsync.header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC;
    vsync.header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC;
    vsync.header.timestamp = timestamp;
    vsync.header.timestamp = timestamp;
    vsync.vsync.count = mDeliveredEvents;
    vsync.vsync.count = vsyncCount;


    const size_t count = displayEventConnections.size();
    const size_t count = activeConnections.size();
    for (size_t i=0 ; i<count ; i++) {
    for (size_t i=0 ; i<count ; i++) {
        sp<Connection> conn(displayEventConnections[i].promote());
        const sp<Connection>& conn(activeConnections[i]);
        // make sure the connection didn't die
        // now see if we still need to report this VSYNC event
        if (conn != NULL) {
        const int32_t vcount = conn->count;
        if (vcount >= 0) {
            bool reportVsync = false;
            if (vcount == 0) {
                // fired this time around
                conn->count = -1;
                reportVsync = true;
            } else if (vcount == 1 || (vsyncCount % vcount) == 0) {
                // continuous event, and time to report it
                reportVsync = true;
            }

            if (reportVsync) {
                status_t err = conn->postEvent(vsync);
                status_t err = conn->postEvent(vsync);
                if (err == -EAGAIN || err == -EWOULDBLOCK) {
                if (err == -EAGAIN || err == -EWOULDBLOCK) {
                    // The destination doesn't accept events anymore, it's probably
                    // The destination doesn't accept events anymore, it's probably
@@ -231,17 +225,11 @@ bool EventThread::threadLoop() {
                    // handle any other error on the pipe as fatal. the only
                    // handle any other error on the pipe as fatal. the only
                    // reasonable thing to do is to clean-up this connection.
                    // reasonable thing to do is to clean-up this connection.
                    // The most common error we'll get here is -EPIPE.
                    // The most common error we'll get here is -EPIPE.
                removeDisplayEventConnection(displayEventConnections[i]);
                    removeDisplayEventConnection(activeConnections[i]);
                }
            }
            }
        } else {
            // somehow the connection is dead, but we still have it in our list
            // just clean the list.
            removeDisplayEventConnection(displayEventConnections[i]);
        }
        }
    }
    }

    // clear all our references without holding mLock
    displayEventConnections.clear();


    return true;
    return true;
}
}
@@ -273,7 +261,7 @@ void EventThread::dump(String8& result, char* buffer, size_t SIZE) const {
    result.appendFormat("  soft-vsync: %s\n",
    result.appendFormat("  soft-vsync: %s\n",
            mUseSoftwareVSync?"enabled":"disabled");
            mUseSoftwareVSync?"enabled":"disabled");
    result.appendFormat("  numListeners=%u,\n  events-delivered: %u\n",
    result.appendFormat("  numListeners=%u,\n  events-delivered: %u\n",
            mDisplayEventConnections.size(), mDeliveredEvents);
            mDisplayEventConnections.size(), mVSyncCount);
    for (size_t i=0 ; i<mDisplayEventConnections.size() ; i++) {
    for (size_t i=0 ; i<mDisplayEventConnections.size() ; i++) {
        sp<Connection> connection =
        sp<Connection> connection =
                mDisplayEventConnections.itemAt(i).promote();
                mDisplayEventConnections.itemAt(i).promote();
+1 −4
Original line number Original line Diff line number Diff line
@@ -100,12 +100,9 @@ private:


    // protected by mLock
    // protected by mLock
    SortedVector< wp<Connection> > mDisplayEventConnections;
    SortedVector< wp<Connection> > mDisplayEventConnections;
    nsecs_t mLastVSyncTimestamp;
    nsecs_t mVSyncTimestamp;
    nsecs_t mVSyncTimestamp;
    bool mUseSoftwareVSync;
    bool mUseSoftwareVSync;

    size_t mVSyncCount;
    // main thread only
    size_t mDeliveredEvents;


    // for debugging
    // for debugging
    bool mDebugVsyncEnabled;
    bool mDebugVsyncEnabled;