commit 8f835329285941fd27e817d5bae8fe258d51e367
author Jorge E. Moreira <jemoreira@google.com> Mon Mar 19 16:19:38 2018 -0700
committer Jorge Moreira Broche <jemoreira@google.com> Tue Mar 20 00:44:33 2018 +0000
tree acd592f4712d8976e356bef1e05544e125363cce
parent 50778ea12594bae9e1aee82f59e0ce3747416365

Fixes race on hwcomposer vsync thread.

Bug: 75402353
Test: local
Change-Id: I3ed6c8543b39e5ca119cc8db6c019a5f08dbdcf5

guest/hals/hwcomposer/hwcomposer.cpp

diff --git a/guest/hals/hwcomposer/hwcomposer.cpp b/guest/hals/hwcomposer/hwcomposer.cpp
index d510285..fdeef65 100644
--- a/guest/hals/hwcomposer/hwcomposer.cpp
+++ b/guest/hals/hwcomposer/hwcomposer.cpp
@@ -94,6 +94,8 @@
   int sent = 0;
   int last_sent = 0;
   static const int log_interval = 60;
+  void (*vsync_proc)(const struct hwc_procs*, int, int64_t) = nullptr;
+  bool log_no_procs = true, log_no_vsync = true;
   while (true) {
     struct timespec rt;
     if (clock_gettime(CLOCK_MONOTONIC, &rt) == -1) {
@@ -115,13 +117,27 @@
       }
     }
 
-    pdev->procs->vsync(const_cast<hwc_procs_t*>(pdev->procs), 0, timestamp);
+    // The vsync thread is started on device open, it may run before the
+    // registerProcs callback has a chance to be called, so we need to make sure
+    // procs is not NULL before dereferencing it.
+    if (pdev && pdev->procs) {
+      vsync_proc = pdev->procs->vsync;
+    } else if (log_no_procs) {
+      log_no_procs = false;
+      ALOGI("procs is not set yet, unable to deliver vsync event");
+    }
+    if (vsync_proc) {
+      vsync_proc(const_cast<hwc_procs_t*>(pdev->procs), 0, timestamp);
+      ++sent;
+    } else if (log_no_vsync) {
+      log_no_vsync = false;
+      ALOGE("vsync callback is null (but procs was already set)");
+    }
     if (rt.tv_sec - last_logged > log_interval) {
       ALOGI("Sent %d syncs in %ds", sent - last_sent, log_interval);
       last_logged = rt.tv_sec;
       last_sent = sent;
     }
-    ++sent;
   }
 
   return NULL;

guest/hals/hwcomposer/legacy/hwcomposer.cpp

diff --git a/guest/hals/hwcomposer/legacy/hwcomposer.cpp b/guest/hals/hwcomposer/legacy/hwcomposer.cpp
index 29e8f0f..33122eb 100644
--- a/guest/hals/hwcomposer/legacy/hwcomposer.cpp
+++ b/guest/hals/hwcomposer/legacy/hwcomposer.cpp
@@ -355,6 +355,8 @@
   int sent = 0;
   int last_sent = 0;
   static const int log_interval = 60;
+  void (*vsync_proc)(const struct hwc_procs*, int, int64_t) = nullptr;
+  bool log_no_procs = true, log_no_vsync = true;
   while (true) {
     struct timespec rt;
     if (clock_gettime(CLOCK_MONOTONIC, &rt) == -1) {
@@ -376,13 +378,27 @@
       }
     }
 
-    pdev->procs->vsync(const_cast<hwc_procs_t*>(pdev->procs), 0, timestamp);
+    // The vsync thread is started on device open, it may run before the
+    // registerProcs callback has a chance to be called, so we need to make sure
+    // procs is not NULL before dereferencing it.
+    if (pdev && pdev->procs) {
+      vsync_proc = pdev->procs->vsync;
+    } else if (log_no_procs) {
+      log_no_procs = false;
+      ALOGI("procs is not set yet, unable to deliver vsync event");
+    }
+    if (vsync_proc) {
+      vsync_proc(const_cast<hwc_procs_t*>(pdev->procs), 0, timestamp);
+      ++sent;
+    } else if (log_no_vsync) {
+      log_no_vsync = false;
+      ALOGE("vsync callback is null (but procs was already set)");
+    }
     if (rt.tv_sec - last_logged > log_interval) {
       ALOGI("Sent %d syncs in %ds", sent - last_sent, log_interval);
       last_logged = rt.tv_sec;
       last_sent = sent;
     }
-    ++sent;
   }
 
   return NULL;