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>

6 files changed, 10 insertions, 23 deletions

diff --git a/drm/ResourceManager.cpp b/drm/ResourceManager.cpp
index f64cae4..dd26c17 100644
--- a/drm/ResourceManager.cpp
+++ b/drm/ResourceManager.cpp
@@ -82,7 +82,7 @@ void ResourceManager::Init() {
   }
 
   uevent_listener_->RegisterHotplugHandler([this] {
-    const std::lock_guard<std::mutex> lock(GetMainLock());
+    const std::unique_lock lock(GetMainLock());
     UpdateFrontendDisplays();
   });
 
diff --git a/drm/ResourceManager.h b/drm/ResourceManager.h
index 8e047a3..60b6b16 100644
--- a/drm/ResourceManager.h
+++ b/drm/ResourceManager.h
@@ -69,7 +69,7 @@ class ResourceManager {
 
   std::shared_ptr<UEventListener> uevent_listener_;
 
-  std::mutex main_lock_;
+  std::recursive_mutex main_lock_;
 
   std::map<DrmConnector *, std::unique_ptr<DrmDisplayPipeline>>
       attached_pipelines_;
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 @@ HWC2::Error DrmHwcTwo::RegisterCallback(int32_t descriptor,
         /* 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 @@ HWC2::Error DrmHwcTwo::RegisterCallback(int32_t descriptor,
 }
 
 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();
   }
 }
 
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 @@ class DrmHwcTwo : public PipelineToFrontendBindingInterface {
                                                  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_;
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 @@ HWC2::Error HwcDisplay::Init() {
   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);
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 @@ template <typename T, typename HookType, HookType func, typename... Args>
 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 @@ static int32_t DisplayHook(hwc2_device_t *dev, hwc2_display_t display_handle,
   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 @@ static int32_t LayerHook(hwc2_device_t *dev, hwc2_display_t display_handle,
   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);