commit 9e2a2cd3132eae959293e94d32f2ca85653a788e
author Roman Stratiienko <r.stratiienko@gmail.com> Wed Dec 28 20:47:29 2022 +0200
committer Roman Stratiienko <r.stratiienko@gmail.com> Wed Jan 04 16:30:40 2023 +0200
tree f0e006ae8d7f4f7111bb1742e480fd04d3dfce6d
parent f818d4c92c407cb6460c89ab06eceeed4f5aa468

drm_hwcomposer: Make main mutex recursive

It allows to remove redundant unlock/lock pair from the code,
and should make it a little bit more race-proof.

Signed-off-by: Roman Stratiienko <r.stratiienko@gmail.com>

hwc2_device/DrmHwcTwo.cpp

diff --git a/hwc2_device/DrmHwcTwo.cpp b/hwc2_device/DrmHwcTwo.cpp
index 707319e..64755c3 100644
--- a/hwc2_device/DrmHwcTwo.cpp
+++ b/hwc2_device/DrmHwcTwo.cpp
@@ -173,10 +173,7 @@
         /* Headless display may still be here. Remove it! */
         if (displays_.count(kPrimaryDisplay) != 0) {
           displays_[kPrimaryDisplay]->Deinit();
-          auto &mutex = GetResMan().GetMainLock();
-          mutex.unlock();
           displays_.erase(kPrimaryDisplay);
-          mutex.lock();
         }
       }
       break;
@@ -207,24 +204,15 @@
 }
 
 void DrmHwcTwo::SendHotplugEventToClient(hwc2_display_t displayid,
-                                         bool connected) {
-  auto &mutex = GetResMan().GetMainLock();
-  if (mutex.try_lock()) {
-    ALOGE("FIXME!!!: Main mutex must be locked in %s", __func__);
-    mutex.unlock();
-    return;
-  }
-
+                                         bool connected) const {
   auto hc = hotplug_callback_;
   if (hc.first != nullptr && hc.second != nullptr) {
-    /* For some reason CLIENT will call HWC2 API in hotplug callback handler,
-     * which will cause deadlock . Unlock main mutex to prevent this.
+    /* For some reason HWC Service will call HWC2 API in hotplug callback
+     * handler. This is the reason we're using recursive mutex.
      */
-    mutex.unlock();
     hc.first(hc.second, displayid,
              connected == DRM_MODE_CONNECTED ? HWC2_CONNECTION_CONNECTED
                                              : HWC2_CONNECTION_DISCONNECTED);
-    mutex.lock();
   }
 }
 

hwc2_device/DrmHwcTwo.h

diff --git a/hwc2_device/DrmHwcTwo.h b/hwc2_device/DrmHwcTwo.h
index 7a65853..81c5155 100644
--- a/hwc2_device/DrmHwcTwo.h
+++ b/hwc2_device/DrmHwcTwo.h
@@ -71,7 +71,7 @@
                                                  int64_t timestamp) const;
 
  private:
-  void SendHotplugEventToClient(hwc2_display_t displayid, bool connected);
+  void SendHotplugEventToClient(hwc2_display_t displayid, bool connected) const;
 
   ResourceManager resource_manager_;
   std::map<hwc2_display_t, std::unique_ptr<HwcDisplay>> displays_;

hwc2_device/HwcDisplay.cpp

diff --git a/hwc2_device/HwcDisplay.cpp b/hwc2_device/HwcDisplay.cpp
index 4948755..194889e 100644
--- a/hwc2_device/HwcDisplay.cpp
+++ b/hwc2_device/HwcDisplay.cpp
@@ -155,8 +155,7 @@
   auto vsw_callbacks = (VSyncWorkerCallbacks){
       .out_event =
           [this](int64_t timestamp) {
-            const std::lock_guard<std::mutex> lock(
-                hwc2_->GetResMan().GetMainLock());
+            const std::unique_lock lock(hwc2_->GetResMan().GetMainLock());
             if (vsync_event_en_) {
               uint32_t period_ns{};
               GetDisplayVsyncPeriod(&period_ns);

hwc2_device/hwc2_device.cpp

diff --git a/hwc2_device/hwc2_device.cpp b/hwc2_device/hwc2_device.cpp
index 87a7705..d4ee10d 100644
--- a/hwc2_device/hwc2_device.cpp
+++ b/hwc2_device/hwc2_device.cpp
@@ -63,7 +63,7 @@
 static T DeviceHook(hwc2_device_t *dev, Args... args) {
   ALOGV("Device hook: %s", GetFuncName(__PRETTY_FUNCTION__).c_str());
   DrmHwcTwo *hwc = ToDrmHwcTwo(dev);
-  const std::lock_guard<std::mutex> lock(hwc->GetResMan().GetMainLock());
+  const std::unique_lock lock(hwc->GetResMan().GetMainLock());
   return static_cast<T>(((*hwc).*func)(std::forward<Args>(args)...));
 }
 
@@ -73,7 +73,7 @@
   ALOGV("Display #%" PRIu64 " hook: %s", display_handle,
         GetFuncName(__PRETTY_FUNCTION__).c_str());
   DrmHwcTwo *hwc = ToDrmHwcTwo(dev);
-  const std::lock_guard<std::mutex> lock(hwc->GetResMan().GetMainLock());
+  const std::unique_lock lock(hwc->GetResMan().GetMainLock());
   auto *display = hwc->GetDisplay(display_handle);
   if (display == nullptr)
     return static_cast<int32_t>(HWC2::Error::BadDisplay);
@@ -87,7 +87,7 @@
   ALOGV("Display #%" PRIu64 " Layer: #%" PRIu64 " hook: %s", display_handle,
         layer_handle, GetFuncName(__PRETTY_FUNCTION__).c_str());
   DrmHwcTwo *hwc = ToDrmHwcTwo(dev);
-  const std::lock_guard<std::mutex> lock(hwc->GetResMan().GetMainLock());
+  const std::unique_lock lock(hwc->GetResMan().GetMainLock());
   auto *display = hwc->GetDisplay(display_handle);
   if (display == nullptr)
     return static_cast<int32_t>(HWC2::Error::BadDisplay);