commit 30f4e9c5938a906120ff13260acde9bdcc4317b0
author Drew Davenport <ddavenport@google.com> Mon Dec 16 16:12:11 2024 -0700
committer Drew Davenport <ddavenport@google.com> Mon Dec 16 17:53:54 2024 -0700
tree 30050f7d4029dc69be7f8f0f51338910bdc9e4e4
parent 15016c496c156efeab566d15a2582e3ab02c0cc1

drm_hwcomposer: Clean up VsyncWorker destruction

With VsyncWorker no longer acquiring the main lock, we can use
std::thread::join on the main thread to ensure that the vsync thread is
stopped cleanly in the VSyncWorker destructor.

- Don't pass a shared_ptr<> into VSyncWorker::ThreadFn to simplify
  lifetime management
- Change to unique_ptr to simplify lifetime management
- Remove the hack to destroy the HwcDisplays in two stages when
  destructing ComposerClient
- Add std::thread::join to VSyncWorker destructor

Change-Id: I25a34fd304c7b2ec48e43d538bf15794bdc9d68e
Signed-off-by: Drew Davenport <ddavenport@google.com>

drm/VSyncWorker.cpp

diff --git a/drm/VSyncWorker.cpp b/drm/VSyncWorker.cpp
index ac80da4..64152ab 100644
--- a/drm/VSyncWorker.cpp
+++ b/drm/VSyncWorker.cpp
@@ -31,8 +31,8 @@
 namespace android {
 
 auto VSyncWorker::CreateInstance(std::shared_ptr<DrmDisplayPipeline> &pipe)
-    -> std::shared_ptr<VSyncWorker> {
-  auto vsw = std::shared_ptr<VSyncWorker>(new VSyncWorker());
+    -> std::unique_ptr<VSyncWorker> {
+  auto vsw = std::unique_ptr<VSyncWorker>(new VSyncWorker());
 
   if (pipe) {
     vsw->high_crtc_ = pipe->crtc->Get()->GetIndexInResArray()
@@ -40,11 +40,17 @@
     vsw->drm_fd_ = pipe->device->GetFd();
   }
 
-  std::thread(&VSyncWorker::ThreadFn, vsw.get(), vsw).detach();
+  vsw->vswt_ = std::thread(&VSyncWorker::ThreadFn, vsw.get());
 
   return vsw;
 }
 
+VSyncWorker::~VSyncWorker() {
+  StopThread();
+
+  vswt_.join();
+}
+
 void VSyncWorker::UpdateVSyncControl() {
   {
     const std::lock_guard<std::mutex> lock(mutex_);
@@ -143,17 +149,17 @@
   return 0;
 }
 
-void VSyncWorker::ThreadFn(const std::shared_ptr<VSyncWorker> &vsw) {
+void VSyncWorker::ThreadFn() {
   int ret = 0;
 
   for (;;) {
     {
-      std::unique_lock<std::mutex> lock(vsw->mutex_);
+      std::unique_lock<std::mutex> lock(mutex_);
       if (thread_exit_)
         break;
 
       if (!enabled_)
-        vsw->cv_.wait(lock);
+        cv_.wait(lock);
 
       if (!enabled_)
         continue;

drm/VSyncWorker.h

diff --git a/drm/VSyncWorker.h b/drm/VSyncWorker.h
index 0809f48..c76dd14 100644
--- a/drm/VSyncWorker.h
+++ b/drm/VSyncWorker.h
@@ -31,10 +31,10 @@
   using VsyncTimestampCallback = std::function<void(int64_t /*timestamp*/,
                                                     uint32_t /*period*/)>;
 
-  ~VSyncWorker() = default;
+  ~VSyncWorker();
 
   auto static CreateInstance(std::shared_ptr<DrmDisplayPipeline> &pipe)
-      -> std::shared_ptr<VSyncWorker>;
+      -> std::unique_ptr<VSyncWorker>;
 
   // Set the expected vsync period.
   void SetVsyncPeriodNs(uint32_t vsync_period_ns);
@@ -53,7 +53,7 @@
  private:
   VSyncWorker() = default;
 
-  void ThreadFn(const std::shared_ptr<VSyncWorker> &vsw);
+  void ThreadFn();
 
   int64_t GetPhasedVSync(int64_t frame_ns, int64_t current) const;
   int SyntheticWaitVBlank(int64_t *timestamp);