commit 2274a3146953176b72f2cf2cc578945d8a95f2ff
author Arthur Hung <arthurhung@google.com> Wed Apr 28 15:13:06 2021 +0000
committer Arthur Hung <arthurhung@google.com> Mon May 03 08:44:58 2021 +0000
tree 94bc623d3a3555f06163ce453926a12278b6e615
parent 0021fac86976361657fbc74bc908d548c18511ea

Fix potential synchronized race between SF and InputFlinger

The 'CountDownLatch' is used to control the synchronization behavior if
a transaction is synchronous or contains the syncInputWindow command,
while updating input windows asynchronously, if there is another
synchronized transaction queued and commited before input windows
updated, it may cause the transaction return early and cause other input
function such as 'transferTouchFocus' failed.

This CL distinguish the transaction and input windows updating
synchronization that could help to signal the CountDownLatch in correct
situation.

Test: atest SurfaceFlinger_tests
Test: atest CrossDragAndDropTests --rerun-until-failure 50
Bug: 183877670
Bug: 183877512
Bug: 183880412
Change-Id: I68e124006e86d8b9c4b8c528418b9db2065b21de

services/surfaceflinger/SurfaceFlinger.cpp

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 6497919..df98eb6 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3105,7 +3105,7 @@
 
 void SurfaceFlinger::commitTransaction() {
     commitTransactionLocked();
-    signalSynchronousTransactions();
+    signalSynchronousTransactions(CountDownLatch::eSyncTransaction);
     mAnimTransactionPending = false;
 }
 
@@ -3527,7 +3527,9 @@
     // Generate a CountDownLatch pending state if this is a synchronous transaction.
     if ((state.flags & eSynchronous) || state.inputWindowCommands.syncInputWindows) {
         state.transactionCommittedSignal = std::make_shared<CountDownLatch>(
-                (state.inputWindowCommands.syncInputWindows ? 2 : 1));
+                (state.inputWindowCommands.syncInputWindows
+                         ? (CountDownLatch::eSyncInputWindows | CountDownLatch::eSyncTransaction)
+                         : CountDownLatch::eSyncTransaction));
     }
 
     mTransactionQueue.emplace(state);
@@ -3552,10 +3554,10 @@
     }
 }
 
-void SurfaceFlinger::signalSynchronousTransactions() {
+void SurfaceFlinger::signalSynchronousTransactions(const uint32_t flag) {
     for (auto it = mTransactionCommittedSignals.begin();
          it != mTransactionCommittedSignals.end();) {
-        if ((*it)->countDown() == 0) {
+        if ((*it)->countDown(flag)) {
             it = mTransactionCommittedSignals.erase(it);
         } else {
             it++;
@@ -6175,7 +6177,7 @@
 
 void SurfaceFlinger::setInputWindowsFinished() {
     Mutex::Autolock _l(mStateLock);
-    signalSynchronousTransactions();
+    signalSynchronousTransactions(CountDownLatch::eSyncInputWindows);
 }
 
 // ---------------------------------------------------------------------------