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

Commit bd589e32 authored by Mathias Agopian's avatar Mathias Agopian Committed by The Android Automerger
Browse files

fix various issues in SF's EventThread 

- one issues caused most timestamps to be reported as 0
- on rare occasions an uninitialized variable could be used
- vsync counts per connection were accessed unthreadsafely

we now have 2 lists of connections in the main loop, one just
keeps a list of strong refs to the connections because once
we have a strong ref we're not allowed to release it while
holding the lock.

the 2nd list holds the connections that have a vsync event to
be reported. all the calculations are made with the lock held.

Change-Id: Iacfad3745b05df79d9ece3719bd4c34ddbfd5b83
parent d77d6acc

services/surfaceflinger/EventThread.cpp

+51 −50
Original line number Diff line number Diff line
@@ -19,6 +19,8 @@ 
#include <stdint.h>

#include <sys/types.h>



#include <cutils/compiler.h>



#include <gui/BitTube.h>

#include <gui/IDisplayEventConnection.h>

#include <gui/DisplayEventReceiver.h>
@@ -123,35 +125,53 @@ bool EventThread::threadLoop() { 


    nsecs_t timestamp;

    size_t vsyncCount;

    size_t activeEvents;

    DisplayEventReceiver::Event vsync;

    Vector< sp<EventThread::Connection> > activeConnections;

    Vector< sp<EventThread::Connection> > signalConnections;



    do {

        // release our references

        signalConnections.clear();

        activeConnections.clear();



        Mutex::Autolock _l(mLock);



        // latch VSYNC event if any

        bool waitForVSync = false;

        vsyncCount = mVSyncCount;

        timestamp = mVSyncTimestamp;

        mVSyncTimestamp = 0;



        // check if we should be waiting for VSYNC events

        activeEvents = 0;

        bool waitForNextVsync = false;

        // find out connections waiting for VSYNC events

        size_t count = mDisplayEventConnections.size();

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

            sp<Connection> connection(mDisplayEventConnections[i].promote());

            if (connection != NULL) {

                activeConnections.add(connection);

                if (connection->count >= 0) {

                    // at least one continuous mode or active one-shot event

                    waitForNextVsync = true;

                    activeEvents++;

                    break;

                    // we need vsync events because at least

                    // one connection is waiting for it

                    waitForVSync = true;

                    if (connection->count == 0) {

                        // fired this time around

                        if (timestamp) {

                            // only "consume" this event if we're going to

                            // report it

                            connection->count = -1;

                        }

                        signalConnections.add(connection);

                    } else if (connection->count == 1 ||

                            (vsyncCount % connection->count) == 0) {

                        // continuous event, and time to report it

                        signalConnections.add(connection);

                    }

                }

            }

        }



        if (timestamp) {

            if (!waitForNextVsync) {

            // we have a vsync event we can dispatch

            if (!waitForVSync) {

                // we received a VSYNC but we have no clients

                // don't report it, and disable VSYNC events

                disableVSyncLocked();
@@ -164,14 +184,14 @@ bool EventThread::threadLoop() { 
            // we'll wait to receive the event and we'll

            // reevaluate whether we need to dispatch it and/or

            // disable VSYNC events then.

            if (waitForNextVsync) {

            if (waitForVSync) {

                // enable

                enableVSyncLocked();

            }

        }



        // wait for something to happen

        if (mUseSoftwareVSync && waitForNextVsync) {

        if (CC_UNLIKELY(mUseSoftwareVSync && waitForVSync)) {

            // h/w vsync cannot be used (screen is off), so we use

            // a  timeout instead. it doesn't matter how imprecise this

            // is, we just need to make sure to serve the clients
@@ -180,40 +200,23 @@ bool EventThread::threadLoop() { 
                mVSyncCount++;

            }

        } else {

            if (!timestamp || signalConnections.isEmpty()) {

                // This is where we spend most of our time, waiting

                // for a vsync events and registered clients

                mCondition.wait(mLock);

            }

        vsyncCount = mVSyncCount;

    } while (!activeConnections.size());



    if (!activeEvents) {

        // no events to return. start over.

        // (here we make sure to exit the scope of this function

        //  so that we release our Connection references)

        return true;

        }

    } while (!timestamp || signalConnections.isEmpty());



    // dispatch vsync events to listeners...

    vsync.header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC;

    vsync.header.timestamp = timestamp;

    vsync.vsync.count = vsyncCount;



    const size_t count = activeConnections.size();

    const size_t count = signalConnections.size();

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

        const sp<Connection>& conn(activeConnections[i]);

        const sp<Connection>& conn(signalConnections[i]);

        // now see if we still need to report this VSYNC event

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

        if (err == -EAGAIN || err == -EWOULDBLOCK) {

            // The destination doesn't accept events anymore, it's probably
@@ -225,9 +228,7 @@ 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.

                    removeDisplayEventConnection(activeConnections[i]);

                }

            }

            removeDisplayEventConnection(signalConnections[i]);

        }

    }

services/surfaceflinger/EventThread.h

+0 −1
Original line number Diff line number Diff line
@@ -47,7 +47,6 @@ class EventThread : public Thread { 
        // count >= 1 : continuous event. count is the vsync rate

        // count == 0 : one-shot event that has not fired

        // count ==-1 : one-shot event that fired this round / disabled

        // count ==-2 : one-shot event that fired the round before

        int32_t count;



    private: