From 69c3d450cb32fab6c7f16df53f31a743b8ba5cf0 Mon Sep 17 00:00:00 2001
From: Xiang Wang <xwxw@google.com>
Date: Thu, 26 Jan 2023 15:16:08 -0800
Subject: [PATCH] Reconnect Thermal AIDL HAL on failure

Initialize callback on wrapper creation, otherwise it may race with
the immediate callback from the Thermal HAL and result in NPE

Bug: b/205762943
Test: atest ThermalManagerServiceMockingTest + service restart test
Change-Id: I3648cd628dbdc05b57ad33f52b20c7ee2f201f65
---
 .../server/power/ThermalManagerService.java   | 37 ++++++++++++++-----
 .../ThermalManagerServiceMockingTest.java     |  3 +-
 2 files changed, 29 insertions(+), 11 deletions(-)

diff --git a/services/core/java/com/android/server/power/ThermalManagerService.java b/services/core/java/com/android/server/power/ThermalManagerService.java
index 514caf25dcd0d..58c814a7ad3e6 100644
--- a/services/core/java/com/android/server/power/ThermalManagerService.java
+++ b/services/core/java/com/android/server/power/ThermalManagerService.java
@@ -129,6 +129,9 @@ public class ThermalManagerService extends SystemService {
     ThermalManagerService(Context context, @Nullable ThermalHalWrapper halWrapper) {
         super(context);
         mHalWrapper = halWrapper;
+        if (halWrapper != null) {
+            halWrapper.setCallback(this::onTemperatureChangedCallback);
+        }
         mStatus = Temperature.THROTTLING_NONE;
     }
 
@@ -149,22 +152,21 @@ public class ThermalManagerService extends SystemService {
             // Connect to HAL and post to listeners.
             boolean halConnected = (mHalWrapper != null);
             if (!halConnected) {
-                mHalWrapper = new ThermalHalAidlWrapper();
+                mHalWrapper = new ThermalHalAidlWrapper(this::onTemperatureChangedCallback);
                 halConnected = mHalWrapper.connectToHal();
             }
             if (!halConnected) {
-                mHalWrapper = new ThermalHal20Wrapper();
+                mHalWrapper = new ThermalHal20Wrapper(this::onTemperatureChangedCallback);
                 halConnected = mHalWrapper.connectToHal();
             }
             if (!halConnected) {
-                mHalWrapper = new ThermalHal11Wrapper();
+                mHalWrapper = new ThermalHal11Wrapper(this::onTemperatureChangedCallback);
                 halConnected = mHalWrapper.connectToHal();
             }
             if (!halConnected) {
-                mHalWrapper = new ThermalHal10Wrapper();
+                mHalWrapper = new ThermalHal10Wrapper(this::onTemperatureChangedCallback);
                 halConnected = mHalWrapper.connectToHal();
             }
-            mHalWrapper.setCallback(this::onTemperatureChangedCallback);
             if (!halConnected) {
                 Slog.w(TAG, "No Thermal HAL service on this device");
                 return;
@@ -723,6 +725,10 @@ public class ThermalManagerService extends SystemService {
             }
         };
 
+        ThermalHalAidlWrapper(TemperatureChangedCallback callback) {
+            mCallback = callback;
+        }
+
         @Override
         protected List<Temperature> getCurrentTemperatures(boolean shouldFilter,
                 int type) {
@@ -832,9 +838,10 @@ public class ThermalManagerService extends SystemService {
                         binder.linkToDeath(this, 0);
                     } catch (RemoteException e) {
                         Slog.e(TAG, "Unable to connect IThermal AIDL instance", e);
-                        mInstance = null;
+                        connectToHal();
                     }
                     if (mInstance != null) {
+                        Slog.i(TAG, "Thermal HAL AIDL service connected.");
                         registerThermalChangedCallback();
                     }
                 }
@@ -850,7 +857,7 @@ public class ThermalManagerService extends SystemService {
                         e);
             } catch (RemoteException e) {
                 Slog.e(TAG, "Unable to connect IThermal AIDL instance", e);
-                mInstance = null;
+                connectToHal();
             }
         }
 
@@ -866,8 +873,8 @@ public class ThermalManagerService extends SystemService {
 
         @Override
         public synchronized void binderDied() {
-            Slog.w(TAG, "IThermal HAL instance died");
-            mInstance = null;
+            Slog.w(TAG, "Thermal AIDL HAL died, reconnecting...");
+            connectToHal();
         }
     }
 
@@ -876,6 +883,10 @@ public class ThermalManagerService extends SystemService {
         @GuardedBy("mHalLock")
         private android.hardware.thermal.V1_0.IThermal mThermalHal10 = null;
 
+        ThermalHal10Wrapper(TemperatureChangedCallback callback) {
+            mCallback = callback;
+        }
+
         @Override
         protected List<Temperature> getCurrentTemperatures(boolean shouldFilter,
                 int type) {
@@ -1011,6 +1022,10 @@ public class ThermalManagerService extends SystemService {
                     }
                 };
 
+        ThermalHal11Wrapper(TemperatureChangedCallback callback) {
+            mCallback = callback;
+        }
+
         @Override
         protected List<Temperature> getCurrentTemperatures(boolean shouldFilter,
                 int type) {
@@ -1145,6 +1160,10 @@ public class ThermalManagerService extends SystemService {
                     }
                 };
 
+        ThermalHal20Wrapper(TemperatureChangedCallback callback) {
+            mCallback = callback;
+        }
+
         @Override
         protected List<Temperature> getCurrentTemperatures(boolean shouldFilter,
                 int type) {
diff --git a/services/tests/mockingservicestests/src/com/android/server/power/ThermalManagerServiceMockingTest.java b/services/tests/mockingservicestests/src/com/android/server/power/ThermalManagerServiceMockingTest.java
index c85ed26976c42..0ae8dfdce6cc0 100644
--- a/services/tests/mockingservicestests/src/com/android/server/power/ThermalManagerServiceMockingTest.java
+++ b/services/tests/mockingservicestests/src/com/android/server/power/ThermalManagerServiceMockingTest.java
@@ -61,8 +61,7 @@ public class ThermalManagerServiceMockingTest {
         mAidlBinder.attachInterface(mAidlHalMock, IThermal.class.getName());
         mTemperatureFuture = new CompletableFuture<>();
         mTemperatureCallback = temperature -> mTemperatureFuture.complete(temperature);
-        mAidlWrapper = new ThermalManagerService.ThermalHalAidlWrapper();
-        mAidlWrapper.setCallback(mTemperatureCallback);
+        mAidlWrapper = new ThermalManagerService.ThermalHalAidlWrapper(mTemperatureCallback);
         mAidlWrapper.initProxyAndRegisterCallback(mAidlBinder);
     }