From bd39b49f11289c158902668eb1175b0cf905b86c Mon Sep 17 00:00:00 2001 From: Svet Ganov Date: Fri, 8 Jun 2018 15:03:46 -0700 Subject: [PATCH 1/2] Handle race between UID state and activty resume There is a race between the UID turning active and activity being resume. The proper fix is very risky, so we temporary add some polling which should hapen pretty rarely anyway as the race is hard to hit. Test: camera works fine cts-tradefed run cts-dev -m CtsCameraTestCases bug:77827041 Change-Id: I1768d00dc08e6e49cec0099f0a29b7889f9affad Merged-In: I1768d00dc08e6e49cec0099f0a29b7889f9affad --- .../camera/libcameraservice/CameraService.cpp | 26 ++++++++++++++++++- 1 file changed, 25 insertions(+), 1 deletion(-) diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp index 282871bc44..c41de8286c 100644 --- a/services/camera/libcameraservice/CameraService.cpp +++ b/services/camera/libcameraservice/CameraService.cpp @@ -55,6 +55,7 @@ #include #include #include +#include #include #include #include @@ -2433,6 +2434,8 @@ bool CameraService::UidPolicy::isUidActive(uid_t uid, String16 callingPackage) { return isUidActiveLocked(uid, callingPackage); } +static const int kPollUidActiveTimeoutMillis = 50; + bool CameraService::UidPolicy::isUidActiveLocked(uid_t uid, String16 callingPackage) { // Non-app UIDs are considered always active // If activity manager is unreachable, assume everything is active @@ -2452,7 +2455,28 @@ bool CameraService::UidPolicy::isUidActiveLocked(uid_t uid, String16 callingPack ActivityManager am; // Okay to access with a lock held as UID changes are dispatched without // a lock and we are a higher level component. - active = am.isUidActive(uid, callingPackage); + int64_t startTimeMillis = 0; + do { + // TODO: Fix this b/109950150! + // Okay this is a hack. There is a race between the UID turning active and + // activity being resumed. The proper fix is very risky, so we temporary add + // some polling which should happen pretty rarely anyway as the race is hard + // to hit. + active = am.isUidActive(uid, callingPackage); + if (active) { + break; + } + if (startTimeMillis <= 0) { + startTimeMillis = uptimeMillis(); + } + int64_t ellapsedTimeMillis = uptimeMillis() - startTimeMillis; + int64_t remainingTimeMillis = kPollUidActiveTimeoutMillis - ellapsedTimeMillis; + if (remainingTimeMillis <= 0) { + break; + } + usleep(remainingTimeMillis * 1000); + } while (true); + if (active) { // Now that we found out the UID is actually active, cache that mActiveUids.insert(uid); From 91175d92d184944d6da4a12663fff9783374c4a3 Mon Sep 17 00:00:00 2001 From: Eino-Ville Talvala Date: Mon, 20 Aug 2018 10:27:58 -0700 Subject: [PATCH 2/2] Camera service: Extend UID check timeout to 300ms. Also make the timeout sleeping more granular, so that the wait won't always be 300ms in case the initial checks fail. Test: Camera CTS passes Bug: 110840510 Change-Id: I3f0d09913b10526dd27cecca50c111712da82846 Merged-In: I3f0d09913b10526dd27cecca50c111712da82846 --- services/camera/libcameraservice/CameraService.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/services/camera/libcameraservice/CameraService.cpp b/services/camera/libcameraservice/CameraService.cpp index c41de8286c..2bf42b6381 100644 --- a/services/camera/libcameraservice/CameraService.cpp +++ b/services/camera/libcameraservice/CameraService.cpp @@ -2434,7 +2434,8 @@ bool CameraService::UidPolicy::isUidActive(uid_t uid, String16 callingPackage) { return isUidActiveLocked(uid, callingPackage); } -static const int kPollUidActiveTimeoutMillis = 50; +static const int64_t kPollUidActiveTimeoutTotalMillis = 300; +static const int64_t kPollUidActiveTimeoutMillis = 50; bool CameraService::UidPolicy::isUidActiveLocked(uid_t uid, String16 callingPackage) { // Non-app UIDs are considered always active @@ -2462,7 +2463,8 @@ bool CameraService::UidPolicy::isUidActiveLocked(uid_t uid, String16 callingPack // activity being resumed. The proper fix is very risky, so we temporary add // some polling which should happen pretty rarely anyway as the race is hard // to hit. - active = am.isUidActive(uid, callingPackage); + active = mActiveUids.find(uid) != mActiveUids.end(); + if (!active) active = am.isUidActive(uid, callingPackage); if (active) { break; } @@ -2470,11 +2472,15 @@ bool CameraService::UidPolicy::isUidActiveLocked(uid_t uid, String16 callingPack startTimeMillis = uptimeMillis(); } int64_t ellapsedTimeMillis = uptimeMillis() - startTimeMillis; - int64_t remainingTimeMillis = kPollUidActiveTimeoutMillis - ellapsedTimeMillis; + int64_t remainingTimeMillis = kPollUidActiveTimeoutTotalMillis - ellapsedTimeMillis; if (remainingTimeMillis <= 0) { break; } + remainingTimeMillis = std::min(kPollUidActiveTimeoutMillis, remainingTimeMillis); + + mUidLock.unlock(); usleep(remainingTimeMillis * 1000); + mUidLock.lock(); } while (true); if (active) {