SupplicantStaIfaceHal: Don't trigger onDeath for remote exception

When supplicant is abruptly killed, any ongoing hwbinder transactions
will raise remote exceptions. The handling of these remote exceptions
might happen before the death notification from hwservicemanager is
processed. To prevent multiple death notifications, don't trigger the
onDeath callback on remote exception handling. Remote exception handling
will just clear internal state and wait for the death notification from
the system to trigger the onDeath callback.

Remote exceptions encountered in the initialization path will still
trigger death notification because we may not get notified from the
service manager.

Bug: 79532177
Test: Unit tests
Test: Crash wpa_supplicant in a loop and ensure that only one death
notification is triggered per crash.
Change-Id: Ia2ca3722aef4c38041a456e8fb3bc9cc60b6241a

2 files changed, 38 insertions, 22 deletions

diff --git a/service/java/com/android/server/wifi/SupplicantStaIfaceHal.java b/service/java/com/android/server/wifi/SupplicantStaIfaceHal.java
index 08a953e6f..5bf44ce66 100644
--- a/service/java/com/android/server/wifi/SupplicantStaIfaceHal.java
+++ b/service/java/com/android/server/wifi/SupplicantStaIfaceHal.java
@@ -332,7 +332,7 @@ public class SupplicantStaIfaceHal {
                 });
             } catch (RemoteException e) {
                 Log.e(TAG, "ISupplicant.listInterfaces exception: " + e);
-                supplicantServiceDiedHandler(ifaceName);
+                handleRemoteException(e, "listInterfaces");
                 return null;
             }
             if (supplicantIfaces.size() == 0) {
@@ -353,7 +353,7 @@ public class SupplicantStaIfaceHal {
                                 });
                     } catch (RemoteException e) {
                         Log.e(TAG, "ISupplicant.getInterface exception: " + e);
-                        supplicantServiceDiedHandler(ifaceName);
+                        handleRemoteException(e, "getInterface");
                         return null;
                     }
                     break;
@@ -386,8 +386,8 @@ public class SupplicantStaIfaceHal {
                             supplicantIface.value = iface;
                         });
             } catch (RemoteException e) {
-                Log.e(TAG, "ISupplicant.createInterface exception: " + e);
-                supplicantServiceDiedHandler(ifaceName);
+                Log.e(TAG, "ISupplicant.addInterface exception: " + e);
+                handleRemoteException(e, "addInterface");
                 return null;
             }
             return supplicantIface.value;
@@ -438,7 +438,7 @@ public class SupplicantStaIfaceHal {
                 }
             } catch (RemoteException e) {
                 Log.e(TAG, "ISupplicant.removeInterface exception: " + e);
-                supplicantServiceDiedHandler(ifaceName);
+                handleRemoteException(e, "removeInterface");
                 return false;
             }
             return true;
@@ -479,13 +479,6 @@ public class SupplicantStaIfaceHal {
         }
     }
 
-    private void supplicantServiceDiedHandler(@NonNull String ifaceName) {
-        synchronized (mLock) {
-            mWifiMonitor.broadcastSupplicantDisconnectionEvent(ifaceName);
-            supplicantServiceDiedHandler();
-        }
-    }
-
     private void supplicantServiceDiedHandler() {
         synchronized (mLock) {
             for (String ifaceName : mISupplicantStaIfaces.keySet()) {
@@ -590,7 +583,7 @@ public class SupplicantStaIfaceHal {
                 return (getSupplicantMockableV1_1() != null);
             } catch (RemoteException e) {
                 Log.e(TAG, "ISupplicant.getService exception: " + e);
-                supplicantServiceDiedHandler();
+                handleRemoteException(e, "getSupplicantMockable");
                 return false;
             }
         }
@@ -2211,7 +2204,7 @@ public class SupplicantStaIfaceHal {
 
     private void handleRemoteException(RemoteException e, String methodStr) {
         synchronized (mLock) {
-            supplicantServiceDiedHandler();
+            clearState();
             Log.e(TAG, "ISupplicantStaIface." + methodStr + " failed with exception", e);
         }
     }
diff --git a/tests/wifitests/src/com/android/server/wifi/SupplicantStaIfaceHalTest.java b/tests/wifitests/src/com/android/server/wifi/SupplicantStaIfaceHalTest.java
index 04af83263..b4ea5af34 100644
--- a/tests/wifitests/src/com/android/server/wifi/SupplicantStaIfaceHalTest.java
+++ b/tests/wifitests/src/com/android/server/wifi/SupplicantStaIfaceHalTest.java
@@ -1401,6 +1401,37 @@ public class SupplicantStaIfaceHalTest {
     }
 
     /**
+     * When wpa_supplicant is dead, we could end up getting a remote exception on a hwbinder call
+     * and then the death notification.
+     */
+    @Test
+    public void testHandleRemoteExceptonAndDeathNotification() throws Exception {
+        executeAndValidateInitializationSequence();
+        assertTrue(mDut.registerDeathHandler(mSupplicantHalDeathHandler));
+        assertTrue(mDut.isInitializationComplete());
+
+        // Throw remote exception on hwbinder call.
+        when(mISupplicantStaIfaceMock.setPowerSave(anyBoolean()))
+                .thenThrow(new RemoteException());
+        assertFalse(mDut.setPowerSave(WLAN0_IFACE_NAME, true));
+        verify(mISupplicantStaIfaceMock).setPowerSave(true);
+
+        // Check that remote exception cleared all internal state.
+        assertFalse(mDut.isInitializationComplete());
+
+        // Ensure that futher calls fail because the remote exception clears any state.
+        assertFalse(mDut.setPowerSave(WLAN0_IFACE_NAME, true));
+        //.. No call to ISupplicantStaIface object
+
+        // Now trigger a death notification and ensure it's handled.
+        assertNotNull(mSupplicantDeathCaptor.getValue());
+        mSupplicantDeathCaptor.getValue().serviceDied(5L);
+
+        // External death notification fires only once!
+        verify(mSupplicantHalDeathHandler).onDeath();
+    }
+
+    /**
      * Tests the setting of log level.
      */
     @Test
@@ -1590,10 +1621,6 @@ public class SupplicantStaIfaceHalTest {
                     .getInterface(any(ISupplicant.IfaceInfo.class),
                             any(ISupplicant.getInterfaceCallback.class));
         }
-        if (causeRemoteException) {
-            mInOrder.verify(mWifiMonitor).broadcastSupplicantDisconnectionEvent(
-                    eq(WLAN0_IFACE_NAME));
-        }
         if (!causeRemoteException && !getZeroInterfaces && !getNullInterface) {
             mInOrder.verify(mISupplicantStaIfaceMock)
                     .registerCallback(any(ISupplicantStaIfaceCallback.class));
@@ -1654,10 +1681,6 @@ public class SupplicantStaIfaceHalTest {
                 .addInterface(any(ISupplicant.IfaceInfo.class),
                         any(android.hardware.wifi.supplicant.V1_1.ISupplicant
                                 .addInterfaceCallback.class));
-        if (causeRemoteException) {
-            mInOrder.verify(mWifiMonitor).broadcastSupplicantDisconnectionEvent(
-                    eq(WLAN0_IFACE_NAME));
-        }
         if (!causeRemoteException && !getNullInterface) {
             mInOrder.verify(mISupplicantStaIfaceMockV1_1)
                     .registerCallback_1_1(