SF: Cleanup EventThread Part 1

The cleanups in this CL are primarily about switching from
android::Thread to std::thread.

1) Convert from android::Thread to std::thread, along with using
std::mutex and std::condition_variable to keep consistency.
2) Switch the header to #pragma once.
3) Added Clang thread annotations and enabled the corresponding warning.
4) Added proper thread shutdown handling (invoked by dtor).

Test: No issues observed on Pixel XL
Bug: None

Change-Id: I2ef0ad18ef75e139f70d856031991f87d187efe6

3 files changed, 131 insertions, 107 deletions

diff --git a/services/surfaceflinger/EventThread.cpp b/services/surfaceflinger/EventThread.cpp
index 6b92cc8a0e..2cbc59edb4 100644
--- a/services/surfaceflinger/EventThread.cpp
+++ b/services/surfaceflinger/EventThread.cpp
@@ -16,10 +16,14 @@
 
 #define ATRACE_TAG ATRACE_TAG_GRAPHICS
 
-#include <stdint.h>
+#include <pthread.h>
+#include <sched.h>
 #include <sys/types.h>
+#include <chrono>
+#include <cstdint>
 
 #include <cutils/compiler.h>
+#include <cutils/sched_policy.h>
 
 #include <gui/DisplayEventReceiver.h>
 #include <gui/IDisplayEventConnection.h>
@@ -31,105 +35,126 @@
 #include "EventThread.h"
 #include "SurfaceFlinger.h"
 
+using namespace std::chrono_literals;
+
 // ---------------------------------------------------------------------------
+
 namespace android {
+
 // ---------------------------------------------------------------------------
 
-EventThread::EventThread(const sp<VSyncSource>& src, SurfaceFlinger& flinger, bool interceptVSyncs)
-      : mVSyncSource(src),
-        mFlinger(flinger),
-        mUseSoftwareVSync(false),
-        mVsyncEnabled(false),
-        mDebugVsyncEnabled(false),
-        mInterceptVSyncs(interceptVSyncs) {
-    for (int32_t i = 0; i < DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES; i++) {
-        mVSyncEvent[i].header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC;
-        mVSyncEvent[i].header.id = 0;
-        mVSyncEvent[i].header.timestamp = 0;
-        mVSyncEvent[i].vsync.count = 0;
+EventThread::EventThread(const sp<VSyncSource>& src, SurfaceFlinger& flinger, bool interceptVSyncs,
+                         const char* threadName)
+      : mVSyncSource(src), mFlinger(flinger), mInterceptVSyncs(interceptVSyncs) {
+    for (auto& event : mVSyncEvent) {
+        event.header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC;
+        event.header.id = 0;
+        event.header.timestamp = 0;
+        event.vsync.count = 0;
+    }
+
+    mThread = std::thread(&EventThread::threadMain, this);
+
+    pthread_setname_np(mThread.native_handle(), threadName);
+
+    pid_t tid = pthread_gettid_np(mThread.native_handle());
+
+    // Use SCHED_FIFO to minimize jitter
+    constexpr int EVENT_THREAD_PRIORITY = 2;
+    struct sched_param param = {0};
+    param.sched_priority = EVENT_THREAD_PRIORITY;
+    if (pthread_setschedparam(mThread.native_handle(), SCHED_FIFO, &param) != 0) {
+        ALOGE("Couldn't set SCHED_FIFO for EventThread");
+    }
+
+    set_sched_policy(tid, SP_FOREGROUND);
+}
+
+EventThread::~EventThread() {
+    {
+        std::lock_guard<std::mutex> lock(mMutex);
+        mKeepRunning = false;
+        mCondition.notify_all();
     }
+    mThread.join();
 }
 
 void EventThread::setPhaseOffset(nsecs_t phaseOffset) {
-    Mutex::Autolock _l(mLock);
+    std::lock_guard<std::mutex> lock(mMutex);
     mVSyncSource->setPhaseOffset(phaseOffset);
 }
 
-void EventThread::onFirstRef() {
-    run("EventThread", PRIORITY_URGENT_DISPLAY + PRIORITY_MORE_FAVORABLE);
-}
-
 sp<EventThread::Connection> EventThread::createEventConnection() const {
     return new Connection(const_cast<EventThread*>(this));
 }
 
 status_t EventThread::registerDisplayEventConnection(
         const sp<EventThread::Connection>& connection) {
-    Mutex::Autolock _l(mLock);
+    std::lock_guard<std::mutex> lock(mMutex);
     mDisplayEventConnections.add(connection);
-    mCondition.broadcast();
+    mCondition.notify_all();
     return NO_ERROR;
 }
 
 void EventThread::removeDisplayEventConnection(const wp<EventThread::Connection>& connection) {
-    Mutex::Autolock _l(mLock);
+    std::lock_guard<std::mutex> lock(mMutex);
     mDisplayEventConnections.remove(connection);
 }
 
 void EventThread::setVsyncRate(uint32_t count, const sp<EventThread::Connection>& connection) {
     if (int32_t(count) >= 0) { // server must protect against bad params
-        Mutex::Autolock _l(mLock);
+        std::lock_guard<std::mutex> lock(mMutex);
         const int32_t new_count = (count == 0) ? -1 : count;
         if (connection->count != new_count) {
             connection->count = new_count;
-            mCondition.broadcast();
+            mCondition.notify_all();
         }
     }
 }
 
 void EventThread::requestNextVsync(const sp<EventThread::Connection>& connection) {
-    Mutex::Autolock _l(mLock);
+    std::lock_guard<std::mutex> lock(mMutex);
 
     mFlinger.resyncWithRateLimit();
 
     if (connection->count < 0) {
         connection->count = 0;
-        mCondition.broadcast();
+        mCondition.notify_all();
     }
 }
 
 void EventThread::onScreenReleased() {
-    Mutex::Autolock _l(mLock);
+    std::lock_guard<std::mutex> lock(mMutex);
     if (!mUseSoftwareVSync) {
         // disable reliance on h/w vsync
         mUseSoftwareVSync = true;
-        mCondition.broadcast();
+        mCondition.notify_all();
     }
 }
 
 void EventThread::onScreenAcquired() {
-    Mutex::Autolock _l(mLock);
+    std::lock_guard<std::mutex> lock(mMutex);
     if (mUseSoftwareVSync) {
         // resume use of h/w vsync
         mUseSoftwareVSync = false;
-        mCondition.broadcast();
+        mCondition.notify_all();
     }
 }
 
 void EventThread::onVSyncEvent(nsecs_t timestamp) {
-    Mutex::Autolock _l(mLock);
+    std::lock_guard<std::mutex> lock(mMutex);
     mVSyncEvent[0].header.type = DisplayEventReceiver::DISPLAY_EVENT_VSYNC;
     mVSyncEvent[0].header.id = 0;
     mVSyncEvent[0].header.timestamp = timestamp;
     mVSyncEvent[0].vsync.count++;
-    mCondition.broadcast();
+    mCondition.notify_all();
 }
 
 void EventThread::onHotplugReceived(int type, bool connected) {
     ALOGE_IF(type >= DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES,
              "received hotplug event for an invalid display (id=%d)", type);
 
-    Mutex::Autolock _l(mLock);
+    std::lock_guard<std::mutex> lock(mMutex);
     if (type < DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES) {
         DisplayEventReceiver::Event event;
         event.header.type = DisplayEventReceiver::DISPLAY_EVENT_HOTPLUG;
@@ -137,46 +162,48 @@ void EventThread::onHotplugReceived(int type, bool connected) {
         event.header.timestamp = systemTime();
         event.hotplug.connected = connected;
         mPendingEvents.add(event);
-        mCondition.broadcast();
+        mCondition.notify_all();
     }
 }
 
-bool EventThread::threadLoop() {
-    DisplayEventReceiver::Event event;
-    Vector<sp<EventThread::Connection> > signalConnections;
-    signalConnections = waitForEvent(&event);
-
-    // dispatch events to listeners...
-    const size_t count = signalConnections.size();
-    for (size_t i = 0; i < count; i++) {
-        const sp<Connection>& conn(signalConnections[i]);
-        // now see if we still need to report this event
-        status_t err = conn->postEvent(event);
-        if (err == -EAGAIN || err == -EWOULDBLOCK) {
-            // The destination doesn't accept events anymore, it's probably
-            // full. For now, we just drop the events on the floor.
-            // FIXME: Note that some events cannot be dropped and would have
-            // to be re-sent later.
-            // Right-now we don't have the ability to do this.
-            ALOGW("EventThread: dropping event (%08x) for connection %p", event.header.type,
-                  conn.get());
-        } else if (err < 0) {
-            // 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(signalConnections[i]);
+void EventThread::threadMain() NO_THREAD_SAFETY_ANALYSIS {
+    std::unique_lock<std::mutex> lock(mMutex);
+    while (mKeepRunning) {
+        DisplayEventReceiver::Event event;
+        Vector<sp<EventThread::Connection> > signalConnections;
+        signalConnections = waitForEventLocked(&lock, &event);
+
+        // dispatch events to listeners...
+        const size_t count = signalConnections.size();
+        for (size_t i = 0; i < count; i++) {
+            const sp<Connection>& conn(signalConnections[i]);
+            // now see if we still need to report this event
+            status_t err = conn->postEvent(event);
+            if (err == -EAGAIN || err == -EWOULDBLOCK) {
+                // The destination doesn't accept events anymore, it's probably
+                // full. For now, we just drop the events on the floor.
+                // FIXME: Note that some events cannot be dropped and would have
+                // to be re-sent later.
+                // Right-now we don't have the ability to do this.
+                ALOGW("EventThread: dropping event (%08x) for connection %p", event.header.type,
+                      conn.get());
+            } else if (err < 0) {
+                // 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(signalConnections[i]);
+            }
         }
     }
-    return true;
 }
 
 // This will return when (1) a vsync event has been received, and (2) there was
 // at least one connection interested in receiving it when we started waiting.
-Vector<sp<EventThread::Connection> > EventThread::waitForEvent(DisplayEventReceiver::Event* event) {
-    Mutex::Autolock _l(mLock);
+Vector<sp<EventThread::Connection> > EventThread::waitForEventLocked(
+        std::unique_lock<std::mutex>* lock, DisplayEventReceiver::Event* event) {
     Vector<sp<EventThread::Connection> > signalConnections;
 
-    do {
+    while (signalConnections.isEmpty() && mKeepRunning) {
         bool eventPending = false;
         bool waitForVSync = false;
 
@@ -279,8 +306,8 @@ Vector<sp<EventThread::Connection> > EventThread::waitForEvent(DisplayEventRecei
                 // use a (long) timeout when waiting for h/w vsync, and
                 // generate fake events when necessary.
                 bool softwareSync = mUseSoftwareVSync;
-                nsecs_t timeout = softwareSync ? ms2ns(16) : ms2ns(1000);
-                if (mCondition.waitRelative(mLock, timeout) == TIMED_OUT) {
+                auto timeout = softwareSync ? 16ms : 1000ms;
+                if (mCondition.wait_for(*lock, timeout) == std::cv_status::timeout) {
                     if (!softwareSync) {
                         ALOGW("Timed out waiting for hw vsync; faking it");
                     }
@@ -296,10 +323,10 @@ Vector<sp<EventThread::Connection> > EventThread::waitForEvent(DisplayEventRecei
                 // h/w vsync should be disabled, so this will wait until we
                 // get a new connection, or an existing connection becomes
                 // interested in receiving vsync again.
-                mCondition.wait(mLock);
+                mCondition.wait(*lock);
             }
         }
-    } while (signalConnections.isEmpty());
+    }
 
     // here we're guaranteed to have a timestamp and some connections to signal
     // (The connections might have dropped out of mDisplayEventConnections
@@ -328,7 +355,7 @@ void EventThread::disableVSyncLocked() {
 }
 
 void EventThread::dump(String8& result) const {
-    Mutex::Autolock _l(mLock);
+    std::lock_guard<std::mutex> lock(mMutex);
     result.appendFormat("VSYNC state: %s\n", mDebugVsyncEnabled ? "enabled" : "disabled");
     result.appendFormat("  soft-vsync: %s\n", mUseSoftwareVSync ? "enabled" : "disabled");
     result.appendFormat("  numListeners=%zu,\n  events-delivered: %u\n",
@@ -377,4 +404,4 @@ status_t EventThread::Connection::postEvent(const DisplayEventReceiver::Event& e
 
 // ---------------------------------------------------------------------------
 
-}; // namespace android
+} // namespace android
diff --git a/services/surfaceflinger/EventThread.h b/services/surfaceflinger/EventThread.h
index b4d718cd17..63cdb542f3 100644
--- a/services/surfaceflinger/EventThread.h
+++ b/services/surfaceflinger/EventThread.h
@@ -14,19 +14,23 @@
  * limitations under the License.
  */
 
-#ifndef ANDROID_SURFACE_FLINGER_EVENT_THREAD_H
-#define ANDROID_SURFACE_FLINGER_EVENT_THREAD_H
+#pragma once
 
 #include <stdint.h>
 #include <sys/types.h>
+#include <condition_variable>
+#include <mutex>
+#include <thread>
+
+#include <android-base/thread_annotations.h>
 
 #include <gui/DisplayEventReceiver.h>
 #include <gui/IDisplayEventConnection.h>
 #include <private/gui/BitTube.h>
 
 #include <utils/Errors.h>
+#include <utils/RefBase.h>
 #include <utils/SortedVector.h>
-#include <utils/threads.h>
 
 #include "DisplayDevice.h"
 
@@ -53,7 +57,7 @@ public:
     virtual void setPhaseOffset(nsecs_t phaseOffset) = 0;
 };
 
-class EventThread : public Thread, private VSyncSource::Callback {
+class EventThread : public virtual RefBase, private VSyncSource::Callback {
     class Connection : public BnDisplayEventConnection {
     public:
         explicit Connection(const sp<EventThread>& eventThread);
@@ -75,7 +79,9 @@ class EventThread : public Thread, private VSyncSource::Callback {
     };
 
 public:
-    EventThread(const sp<VSyncSource>& src, SurfaceFlinger& flinger, bool interceptVSyncs);
+    EventThread(const sp<VSyncSource>& src, SurfaceFlinger& flinger, bool interceptVSyncs,
+                const char* threadName);
+    ~EventThread();
 
     sp<Connection> createEventConnection() const;
     status_t registerDisplayEventConnection(const sp<Connection>& connection);
@@ -92,46 +98,46 @@ public:
     // called when receiving a hotplug event
     void onHotplugReceived(int type, bool connected);
 
-    Vector<sp<EventThread::Connection> > waitForEvent(DisplayEventReceiver::Event* event);
-
     void dump(String8& result) const;
 
     void setPhaseOffset(nsecs_t phaseOffset);
 
 private:
-    virtual bool threadLoop();
-    virtual void onFirstRef();
-
-    virtual void onVSyncEvent(nsecs_t timestamp);
+    void threadMain();
+    Vector<sp<EventThread::Connection>> waitForEventLocked(std::unique_lock<std::mutex>* lock,
+                                                           DisplayEventReceiver::Event* event)
+            REQUIRES(mMutex);
 
     void removeDisplayEventConnection(const wp<Connection>& connection);
-    void enableVSyncLocked();
-    void disableVSyncLocked();
+    void enableVSyncLocked() REQUIRES(mMutex);
+    void disableVSyncLocked() REQUIRES(mMutex);
+
+    // Implements VSyncSource::Callback
+    void onVSyncEvent(nsecs_t timestamp) override;
 
     // constants
-    sp<VSyncSource> mVSyncSource;
+    sp<VSyncSource> mVSyncSource GUARDED_BY(mMutex);
     SurfaceFlinger& mFlinger;
 
-    mutable Mutex mLock;
-    mutable Condition mCondition;
+    std::thread mThread;
+    mutable std::mutex mMutex;
+    mutable std::condition_variable mCondition;
 
     // protected by mLock
-    SortedVector<wp<Connection> > mDisplayEventConnections;
-    Vector<DisplayEventReceiver::Event> mPendingEvents;
-    DisplayEventReceiver::Event mVSyncEvent[DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES];
-    bool mUseSoftwareVSync;
-    bool mVsyncEnabled;
+    SortedVector<wp<Connection>> mDisplayEventConnections GUARDED_BY(mMutex);
+    Vector<DisplayEventReceiver::Event> mPendingEvents GUARDED_BY(mMutex);
+    DisplayEventReceiver::Event mVSyncEvent[DisplayDevice::NUM_BUILTIN_DISPLAY_TYPES] GUARDED_BY(
+            mMutex);
+    bool mUseSoftwareVSync GUARDED_BY(mMutex) = false;
+    bool mVsyncEnabled GUARDED_BY(mMutex) = false;
+    bool mKeepRunning GUARDED_BY(mMutex) = true;
 
     // for debugging
-    bool mDebugVsyncEnabled;
+    bool mDebugVsyncEnabled GUARDED_BY(mMutex) = false;
 
-    const bool mInterceptVSyncs;
+    const bool mInterceptVSyncs = false;
 };
 
 // ---------------------------------------------------------------------------
 
 }; // namespace android
-
-// ---------------------------------------------------------------------------
-
-#endif /* ANDROID_SURFACE_FLINGER_EVENT_THREAD_H */
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 70a282db17..3774ab1e4c 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -579,22 +579,12 @@ void SurfaceFlinger::init() {
     // start the EventThread
     sp<VSyncSource> vsyncSrc =
             new DispSyncSource(&mPrimaryDispSync, SurfaceFlinger::vsyncPhaseOffsetNs, true, "app");
-    mEventThread = new EventThread(vsyncSrc, *this, false);
+    mEventThread = new EventThread(vsyncSrc, *this, false, "appEventThread");
     sp<VSyncSource> sfVsyncSrc =
             new DispSyncSource(&mPrimaryDispSync, SurfaceFlinger::sfVsyncPhaseOffsetNs, true, "sf");
-    mSFEventThread = new EventThread(sfVsyncSrc, *this, true);
+    mSFEventThread = new EventThread(sfVsyncSrc, *this, true, "sfEventThread");
     mEventQueue.setEventThread(mSFEventThread);
 
-    // set EventThread and SFEventThread to SCHED_FIFO to minimize jitter
-    struct sched_param param = {0};
-    param.sched_priority = 2;
-    if (sched_setscheduler(mSFEventThread->getTid(), SCHED_FIFO, &param) != 0) {
-        ALOGE("Couldn't set SCHED_FIFO for SFEventThread");
-    }
-    if (sched_setscheduler(mEventThread->getTid(), SCHED_FIFO, &param) != 0) {
-        ALOGE("Couldn't set SCHED_FIFO for EventThread");
-    }
-
     // Get a RenderEngine for the given display / config (can't fail)
     getBE().mRenderEngine = RenderEngine::create(HAL_PIXEL_FORMAT_RGBA_8888,
             hasWideColorDisplay ? RenderEngine::WIDE_COLOR_SUPPORT : 0);
@@ -1074,7 +1064,8 @@ status_t SurfaceFlinger::enableVSyncInjections(bool enable) {
             ALOGV("VSync Injections enabled");
             if (mVSyncInjector.get() == nullptr) {
                 mVSyncInjector = new InjectVSyncSource();
-                mInjectorEventThread = new EventThread(mVSyncInjector, *this, false);
+                mInjectorEventThread = new EventThread(mVSyncInjector, *this, false,
+                                                       "injEventThread");
             }
             mEventQueue.setEventThread(mInjectorEventThread);
         } else {