commit 14167e00b52f59901e5cff816cf60e7fe2401c88
author Robert Carr <racarr@google.com> Wed Feb 13 13:50:55 2019 -0800
committer Robert Carr <racarr@google.com> Thu Feb 14 13:03:05 2019 -0800
tree 32e2bc63a172c59031961f6f8c9255eceea35295
parent f6d3044c4db6d928a19007bd8ab056c6ee52b7b0

SurfaceFlinger: Avoid calling to SystemServer with lock held.

PermissionCache::checkPermission may make a blocking call in to system-server. We don't
have any lock ordering issues as we end up acquiring the PackageManager lock, and PM
does not call back to SF. However, we can have Binder thread exhaustion issues, in that
many threads can be calling in to the ActivityManager, which could be blocked on the WM,
which could be blocked on SF. If the number of threads calling in to AM is > than our
binder thread pool size we deadlock. I'm not sure we are totally resistant against
thread exhaustion issues but this one is easy enough to fix and there are reports of it.

Bug: 124281288
Test: Existing tests pass
Change-Id: I4ffab2df921478df020bb1089dcd13dd79d99ce6

services/surfaceflinger/SurfaceFlinger.cpp

diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 3abd6a7..08fde8d 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3702,11 +3702,12 @@
         auto& [applyToken, transactionQueue] = *it;
 
         while (!transactionQueue.empty()) {
-            const auto& [states, displays, flags, desiredPresentTime] = transactionQueue.front();
+            const auto& [states, displays, flags, desiredPresentTime, privileged] =
+                    transactionQueue.front();
             if (!transactionIsReadyToBeApplied(desiredPresentTime, states)) {
                 break;
             }
-            applyTransactionState(states, displays, flags, mPendingInputWindowCommands);
+            applyTransactionState(states, displays, flags, mPendingInputWindowCommands, privileged);
             transactionQueue.pop();
         }
 
@@ -3770,6 +3771,9 @@
                                          const InputWindowCommands& inputWindowCommands,
                                          int64_t desiredPresentTime) {
     ATRACE_CALL();
+
+    bool privileged = callingThreadHasUnscopedSurfaceFlingerAccess();
+
     Mutex::Autolock _l(mStateLock);
 
     if (containsAnyInvalidClientState(states)) {
@@ -3779,17 +3783,19 @@
     // If its TransactionQueue already has a pending TransactionState or if it is pending
     if (mTransactionQueues.find(applyToken) != mTransactionQueues.end() ||
         !transactionIsReadyToBeApplied(desiredPresentTime, states)) {
-        mTransactionQueues[applyToken].emplace(states, displays, flags, desiredPresentTime);
+        mTransactionQueues[applyToken].emplace(states, displays, flags, desiredPresentTime,
+                privileged);
         setTransactionFlags(eTransactionNeeded);
         return;
     }
 
-    applyTransactionState(states, displays, flags, inputWindowCommands);
+    applyTransactionState(states, displays, flags, inputWindowCommands, privileged);
 }
 
 void SurfaceFlinger::applyTransactionState(const Vector<ComposerState>& states,
                                            const Vector<DisplayState>& displays, uint32_t flags,
-                                           const InputWindowCommands& inputWindowCommands) {
+                                           const InputWindowCommands& inputWindowCommands,
+                                           bool privileged) {
     uint32_t transactionFlags = 0;
 
     if (flags & eAnimation) {
@@ -3814,7 +3820,7 @@
 
     uint32_t clientStateFlags = 0;
     for (const ComposerState& state : states) {
-        clientStateFlags |= setClientStateLocked(state);
+        clientStateFlags |= setClientStateLocked(state, privileged);
     }
     // If the state doesn't require a traversal and there are callbacks, send them now
     if (!(clientStateFlags & eTraversalNeeded)) {
@@ -3912,7 +3918,7 @@
     return flags;
 }
 
-bool callingThreadHasUnscopedSurfaceFlingerAccess() {
+bool SurfaceFlinger::callingThreadHasUnscopedSurfaceFlingerAccess() {
     IPCThreadState* ipc = IPCThreadState::self();
     const int pid = ipc->getCallingPid();
     const int uid = ipc->getCallingUid();
@@ -3923,7 +3929,8 @@
     return true;
 }
 
-uint32_t SurfaceFlinger::setClientStateLocked(const ComposerState& composerState) {
+uint32_t SurfaceFlinger::setClientStateLocked(const ComposerState& composerState,
+        bool privileged) {
     const layer_state_t& s = composerState.state;
     sp<Client> client(static_cast<Client*>(composerState.client.get()));
 
@@ -4024,7 +4031,7 @@
         // of cropped areas, we need to prevent non-root clients without permission ACCESS_SURFACE_FLINGER
         // (a.k.a. everyone except WindowManager and tests) from setting non rectangle preserving
         // transformations.
-        if (layer->setMatrix(s.matrix, callingThreadHasUnscopedSurfaceFlingerAccess()))
+        if (layer->setMatrix(s.matrix, privileged))
             flags |= eTraversalNeeded;
     }
     if (what & layer_state_t::eTransparentRegionChanged) {