commit 04b3438cf38be98f2d0efeb1a83a5a623e18d242
author Felix Oghina <hackz@google.com> Thu Mar 30 16:29:28 2023 +0000
committer Felix Oghina <hackz@google.com> Tue Apr 11 12:39:57 2023 +0000
tree 5451a18554fbef9183f0af5d914405642797f84d
parent b301516b84ad363a6f392d33febb70fb42c8cc2b

[hotword] init audioflinger outside of lock

The lock used in HotwordDetectionConnection is shared with VoiceInteractionManagerServiceStub, meaning UI thread can end up waiting for audio flinger service to initialize, causing a freeze.

Not that this is not a perfect fix, since the first call (in HotwordDetectionConnection constructor) is still locked. However, the deadlock seems to occur when the service dies and we try to re-initialize, which this change should fix.

Bug: 215887032

Test: atest CtsVoiceInteractionTestCases

Change-Id: I35c682910ac6605041175ab523a71f2b83b499dc

services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordDetectionConnection.java

diff --git a/services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordDetectionConnection.java b/services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordDetectionConnection.java
index f3cb9ba..e6dedc10 100644
--- a/services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordDetectionConnection.java
+++ b/services/voiceinteraction/java/com/android/server/voiceinteraction/HotwordDetectionConnection.java
@@ -153,7 +153,8 @@
     private int mRestartCount = 0;
     @NonNull private ServiceConnection mRemoteHotwordDetectionService;
     @NonNull private ServiceConnection mRemoteVisualQueryDetectionService;
-    private IBinder mAudioFlinger;
+    @GuardedBy("mLock")
+    @Nullable private IBinder mAudioFlinger;
     @GuardedBy("mLock")
     private boolean mDebugHotwordLogging = false;
 
@@ -193,7 +194,7 @@
                 new Intent(VisualQueryDetectionService.SERVICE_INTERFACE);
         visualQueryDetectionServiceIntent.setComponent(mVisualQueryDetectionComponentName);
 
-        initAudioFlingerLocked();
+        initAudioFlinger();
 
         mHotwordDetectionServiceConnectionFactory =
                 new ServiceConnectionFactory(hotwordDetectionServiceIntent,
@@ -226,31 +227,41 @@
         }
     }
 
-    private void initAudioFlingerLocked() {
+    private void initAudioFlinger() {
         if (DEBUG) {
-            Slog.d(TAG, "initAudioFlingerLocked");
+            Slog.d(TAG, "initAudioFlinger");
         }
-        mAudioFlinger = ServiceManager.waitForService("media.audio_flinger");
-        if (mAudioFlinger == null) {
+        final IBinder audioFlinger = ServiceManager.waitForService("media.audio_flinger");
+        if (audioFlinger == null) {
+            setAudioFlinger(null);
             throw new IllegalStateException("Service media.audio_flinger wasn't found.");
         }
         if (DEBUG) {
             Slog.d(TAG, "Obtained audio_flinger binder.");
         }
         try {
-            mAudioFlinger.linkToDeath(mAudioServerDeathRecipient, /* flags= */ 0);
+            audioFlinger.linkToDeath(mAudioServerDeathRecipient, /* flags= */ 0);
         } catch (RemoteException e) {
             Slog.w(TAG, "Audio server died before we registered a DeathRecipient; "
-                            + "retrying init.", e);
-            initAudioFlingerLocked();
+                    + "retrying init.", e);
+            initAudioFlinger();
+            return;
+        }
+
+        setAudioFlinger(audioFlinger);
+    }
+
+    private void setAudioFlinger(@Nullable IBinder audioFlinger) {
+        synchronized (mLock) {
+            mAudioFlinger = audioFlinger;
         }
     }
 
     private void audioServerDied() {
         Slog.w(TAG, "Audio server died; restarting the HotwordDetectionService.");
+        // TODO: Check if this needs to be scheduled on a different thread.
+        initAudioFlinger();
         synchronized (mLock) {
-            // TODO: Check if this needs to be scheduled on a different thread.
-            initAudioFlingerLocked();
             // We restart the process instead of simply sending over the new binder, to avoid race
             // conditions with audio reading in the service.
             restartProcessLocked();