[PATCH hwc v2 14/18] drm_hwcomposer: Fix race in ApplyFrame

[Alexandru Gheorghe <alexandru-cosmin.gheorghe@xxxxxxx>]

ApplyFrame holds the lock just when it swaps the value of
active_composition_, in a multithread context we could end up in a
situation where something is shown on the screen, but something else
is set in active_composition_. Fix it by holding the lock during
CommitFrame.

Signed-off-by: Alexandru Gheorghe <alexandru-cosmin.gheorghe@xxxxxxx>
---
 drmdisplaycompositor.cpp | 40 +++++++++++++++++-----------------------
 drmdisplaycompositor.h   |  2 +-
 2 files changed, 18 insertions(+), 24 deletions(-)

diff --git a/drmdisplaycompositor.cpp b/drmdisplaycompositor.cpp
index afd3b05..576539b 100644
--- a/drmdisplaycompositor.cpp
+++ b/drmdisplaycompositor.cpp
@@ -791,11 +791,6 @@ std::tuple<int, uint32_t> DrmDisplayCompositor::CreateModeBlob(
 }
 
 void DrmDisplayCompositor::ClearDisplay() {
-  AutoLock lock(&lock_, "compositor");
-  int ret = lock.Lock();
-  if (ret)
-    return;
-
   if (!active_composition_)
     return;
 
@@ -808,11 +803,25 @@ void DrmDisplayCompositor::ClearDisplay() {
 }
 
 void DrmDisplayCompositor::ApplyFrame(
-    std::unique_ptr<DrmDisplayComposition> composition, int status) {
+    std::unique_ptr<DrmDisplayComposition> composition, int status,
+    bool writeback) {
+  AutoLock lock(&lock_, __FUNCTION__);
+  if (lock.Lock())
+    return;
   int ret = status;
-
-  if (!ret)
+  if (!ret) {
+    if (writeback && !CountdownExpired()) {
+      ALOGE("Abort playing back scene");
+      return;
+    }
     ret = CommitFrame(composition.get(), false);
+    if (!ret) {
+      ++dump_frames_composited_;
+      if (active_composition_)
+        active_composition_->SignalCompositionDone();
+      active_composition_.swap(composition);
+    }
+  }
 
   if (ret) {
     ALOGE("Composite failed for display %d", display_);
@@ -821,21 +830,6 @@ void DrmDisplayCompositor::ApplyFrame(
     ClearDisplay();
     return;
   }
-  ++dump_frames_composited_;
-
-  if (active_composition_)
-    active_composition_->SignalCompositionDone();
-
-  ret = pthread_mutex_lock(&lock_);
-  if (ret)
-    ALOGE("Failed to acquire lock for active_composition swap");
-
-  active_composition_.swap(composition);
-
-  if (!ret)
-    ret = pthread_mutex_unlock(&lock_);
-  if (ret)
-    ALOGE("Failed to release lock for active_composition swap");
 }
 
 int DrmDisplayCompositor::ApplyComposition(
diff --git a/drmdisplaycompositor.h b/drmdisplaycompositor.h
index 0f8daad..b35ef70 100644
--- a/drmdisplaycompositor.h
+++ b/drmdisplaycompositor.h
@@ -127,7 +127,7 @@ class DrmDisplayCompositor {
 
   void ClearDisplay();
   void ApplyFrame(std::unique_ptr<DrmDisplayComposition> composition,
-                  int status);
+                  int status, bool writeback = false);
 
   std::tuple<int, uint32_t> CreateModeBlob(const DrmMode &mode);
 
-- 
2.7.4

_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel