Recover cleanly from power HAL service crashes

Currently if the binderized power HAL service crashes, services that
call the HAL will crash as well. To fix this:
- Before calling power HAL functions, check that the handle to the
  power HAL service is still valid, and reload using getService() if
  necessary.
- When a call to a power HAL function fails, log the failure and mark
  the handle as invalid.

Bug: 35728909
Test: adb shell kill -9 $(adb shell pidof android.hardware.power@1.0-service)
Change-Id: Id2dd1a6507b9f5044d928483cdb6b736c701f8ba
Signed-off-by: Connor O'Brien <connoro@google.com>
(cherry picked from commit 578eb7fee3a28ff9ca309e741b5a647637896ba1)

2 files changed, 100 insertions, 68 deletions

diff --git a/services/core/jni/com_android_server_am_BatteryStatsService.cpp b/services/core/jni/com_android_server_am_BatteryStatsService.cpp
index ec36df1d9a12..57bb9fedc135 100644
--- a/services/core/jni/com_android_server_am_BatteryStatsService.cpp
+++ b/services/core/jni/com_android_server_am_BatteryStatsService.cpp
@@ -57,6 +57,8 @@ namespace android
 static bool wakeup_init = false;
 static sem_t wakeup_sem;
 extern sp<IPower> gPowerHal;
+extern std::mutex gPowerHalMutex;
+extern bool getPowerHal();
 
 static void wakeup_callback(bool success)
 {
@@ -191,41 +193,26 @@ static jint getPlatformLowPowerStats(JNIEnv* env, jobject /* clazz */, jobject o
         return -1;
     }
 
-    if (gPowerHal == nullptr) {
-        ALOGE("gPowerHal not loaded");
-        return -1;
-    }
+    {
+        std::lock_guard<std::mutex> lock(gPowerHalMutex);
+        if (!getPowerHal()) {
+            ALOGE("Power Hal not loaded");
+            return -1;
+        }
 
-    gPowerHal->getPlatformLowPowerStats(
-        [&offset, &remaining, &total_added](hidl_vec<PowerStatePlatformSleepState> states,
-                Status status) {
-            if (status != Status::SUCCESS)
-                return;
-            for (size_t i = 0; i < states.size(); i++) {
-                int added;
-                const PowerStatePlatformSleepState& state = states[i];
-
-                added = snprintf(offset, remaining,
-                    "state_%zu name=%s time=%" PRIu64 " count=%" PRIu64 " ",
-                    i + 1, state.name.c_str(), state.residencyInMsecSinceBoot,
-                    state.totalTransitions);
-                if (added < 0) {
-                    break;
-                }
-                if (added > remaining) {
-                    added = remaining;
-                }
-                offset += added;
-                remaining -= added;
-                total_added += added;
+        Return<void> ret = gPowerHal->getPlatformLowPowerStats(
+            [&offset, &remaining, &total_added](hidl_vec<PowerStatePlatformSleepState> states,
+                    Status status) {
+                if (status != Status::SUCCESS)
+                    return;
+                for (size_t i = 0; i < states.size(); i++) {
+                    int added;
+                    const PowerStatePlatformSleepState& state = states[i];
 
-                for (size_t j = 0; j < state.voters.size(); j++) {
-                    const PowerStateVoter& voter = state.voters[j];
                     added = snprintf(offset, remaining,
-                            "voter_%zu name=%s time=%" PRIu64 " count=%" PRIu64 " ",
-                            j + 1, voter.name.c_str(),
-                            voter.totalTimeInMsecVotedForSinceBoot,
-                            voter.totalNumberOfTimesVotedSinceBoot);
+                        "state_%zu name=%s time=%" PRIu64 " count=%" PRIu64 " ",
+                        i + 1, state.name.c_str(), state.residencyInMsecSinceBoot,
+                        state.totalTransitions);
                     if (added < 0) {
                         break;
                     }
@@ -235,18 +222,42 @@ static jint getPlatformLowPowerStats(JNIEnv* env, jobject /* clazz */, jobject o
                     offset += added;
                     remaining -= added;
                     total_added += added;
-                }
 
-                if (remaining <= 0) {
-                    /* rewrite NULL character*/
-                    offset--;
-                    total_added--;
-                    ALOGE("PowerHal: buffer not enough");
-                    break;
+                    for (size_t j = 0; j < state.voters.size(); j++) {
+                        const PowerStateVoter& voter = state.voters[j];
+                        added = snprintf(offset, remaining,
+                                "voter_%zu name=%s time=%" PRIu64 " count=%" PRIu64 " ",
+                                j + 1, voter.name.c_str(),
+                                voter.totalTimeInMsecVotedForSinceBoot,
+                                voter.totalNumberOfTimesVotedSinceBoot);
+                        if (added < 0) {
+                            break;
+                        }
+                        if (added > remaining) {
+                            added = remaining;
+                        }
+                        offset += added;
+                        remaining -= added;
+                        total_added += added;
+                    }
+
+                    if (remaining <= 0) {
+                        /* rewrite NULL character*/
+                        offset--;
+                        total_added--;
+                        ALOGE("PowerHal: buffer not enough");
+                        break;
+                    }
                 }
             }
+        );
+
+        if (!ret.isOk()) {
+            ALOGE("getPlatformLowPowerStats() failed: power HAL service not available");
+            gPowerHal = nullptr;
+            return -1;
         }
-    );
+    }
     *offset = 0;
     total_added += 1;
     return total_added;
diff --git a/services/core/jni/com_android_server_power_PowerManagerService.cpp b/services/core/jni/com_android_server_power_PowerManagerService.cpp
index fab309bfb148..1bdcd7aa9bd6 100644
--- a/services/core/jni/com_android_server_power_PowerManagerService.cpp
+++ b/services/core/jni/com_android_server_power_PowerManagerService.cpp
@@ -43,7 +43,7 @@ using android::hardware::Void;
 using android::hardware::power::V1_0::IPower;
 using android::hardware::power::V1_0::PowerHint;
 using android::hardware::power::V1_0::Feature;
-using android::hardware::hidl_vec;
+using android::String8;
 
 namespace android {
 
@@ -56,7 +56,8 @@ static struct {
 // ----------------------------------------------------------------------------
 
 static jobject gPowerManagerServiceObj;
-sp<IPower> gPowerHal;
+sp<IPower> gPowerHal = nullptr;
+std::mutex gPowerHalMutex;
 static nsecs_t gLastEventTime[USER_ACTIVITY_EVENT_LAST + 1];
 
 // Throttling interval for user activity calls.
@@ -74,11 +75,37 @@ static bool checkAndClearExceptionFromCallback(JNIEnv* env, const char* methodNa
     return false;
 }
 
+// Check validity of current handle to the power HAL service, and call getService() if necessary.
+// The caller must be holding gPowerHalMutex.
+bool getPowerHal() {
+    if (gPowerHal == nullptr) {
+        gPowerHal = IPower::getService();
+        if (gPowerHal != nullptr) {
+            ALOGI("Loaded power HAL service");
+        } else {
+            ALOGI("Couldn't load power HAL service");
+        }
+    }
+    return gPowerHal != nullptr;
+}
+
+// Check if a call to a power HAL function failed; if so, log the failure and invalidate the
+// current handle to the power HAL service. The caller must be holding gPowerHalMutex.
+static void processReturn(const Return<void> &ret, const char* functionName) {
+    if (!ret.isOk()) {
+        ALOGE("%s() failed: power HAL service not available.", functionName);
+        gPowerHal = nullptr;
+    }
+}
+
 void android_server_PowerManagerService_userActivity(nsecs_t eventTime, int32_t eventType) {
     // Tell the power HAL when user activity occurs.
-    if (gPowerHal != nullptr) {
-        gPowerHal->powerHint(PowerHint::INTERACTION, 0);
+    gPowerHalMutex.lock();
+    if (getPowerHal()) {
+        Return<void> ret = gPowerHal->powerHint(PowerHint::INTERACTION, 0);
+        processReturn(ret, "powerHint");
     }
+    gPowerHalMutex.unlock();
 
     if (gPowerManagerServiceObj) {
         // Throttle calls into user activity by event type.
@@ -106,14 +133,13 @@ void android_server_PowerManagerService_userActivity(nsecs_t eventTime, int32_t
 }
 
 // ----------------------------------------------------------------------------
-//TODO(b/31632518)
+
 static void nativeInit(JNIEnv* env, jobject obj) {
     gPowerManagerServiceObj = env->NewGlobalRef(obj);
 
-    gPowerHal = IPower::getService();
-    if (gPowerHal == nullptr) {
-        ALOGE("Couldn't load PowerHAL module");
-    }
+    gPowerHalMutex.lock();
+    getPowerHal();
+    gPowerHalMutex.unlock();
 }
 
 static void nativeAcquireSuspendBlocker(JNIEnv *env, jclass /* clazz */, jstring nameStr) {
@@ -127,14 +153,12 @@ static void nativeReleaseSuspendBlocker(JNIEnv *env, jclass /* clazz */, jstring
 }
 
 static void nativeSetInteractive(JNIEnv* /* env */, jclass /* clazz */, jboolean enable) {
-    if (gPowerHal != nullptr) {
-        if (enable) {
-            ALOGD_IF_SLOW(20, "Excessive delay in setInteractive(true) while turning screen on");
-            gPowerHal->setInteractive(true);
-        } else {
-            ALOGD_IF_SLOW(20, "Excessive delay in setInteractive(false) while turning screen off");
-            gPowerHal->setInteractive(false);
-        }
+    std::lock_guard<std::mutex> lock(gPowerHalMutex);
+    if (getPowerHal()) {
+        String8 err("Excessive delay in setInteractive(%s) while turning screen %s");
+        ALOGD_IF_SLOW(20, String8::format(err, enable ? "true" : "false", enable ? "on" : "off"));
+        Return<void> ret = gPowerHal->setInteractive(enable);
+        processReturn(ret, "setInteractive");
     }
 }
 
@@ -149,20 +173,18 @@ static void nativeSetAutoSuspend(JNIEnv* /* env */, jclass /* clazz */, jboolean
 }
 
 static void nativeSendPowerHint(JNIEnv *env, jclass clazz, jint hintId, jint data) {
-    if (gPowerHal != nullptr) {
-        if(data)
-            gPowerHal->powerHint((PowerHint)hintId, data);
-        else {
-            gPowerHal->powerHint((PowerHint)hintId, 0);
-        }
+    std::lock_guard<std::mutex> lock(gPowerHalMutex);
+    if (getPowerHal()) {
+        Return<void> ret =  gPowerHal->powerHint((PowerHint)hintId, data);
+        processReturn(ret, "powerHint");
     }
 }
 
 static void nativeSetFeature(JNIEnv *env, jclass clazz, jint featureId, jint data) {
-    int data_param = data;
-
-    if (gPowerHal != nullptr) {
-        gPowerHal->setFeature((Feature)featureId, data_param ? true : false);
+    std::lock_guard<std::mutex> lock(gPowerHalMutex);
+    if (getPowerHal()) {
+        Return<void> ret = gPowerHal->setFeature((Feature)featureId, static_cast<bool>(data));
+        processReturn(ret, "setFeature");
     }
 }
 
@@ -217,7 +239,6 @@ int register_android_server_PowerManagerService(JNIEnv* env) {
         gLastEventTime[i] = LLONG_MIN;
     }
     gPowerManagerServiceObj = NULL;
-    gPowerHal = NULL;
     return 0;
 }