Merge "Explicitly close sound model file descriptors" into tm-dev

core/java/android/hardware/soundtrigger/SoundTriggerModule.java

@@ -37,6 +37,8 @@ import android.os.Message;
 import android.os.RemoteException;
 import android.util.Log;
 
+import java.io.IOException;
+
 /**
  * The SoundTriggerModule provides APIs to control sound models and sound detection
  * on a given sound trigger hardware module.
@@ -137,13 +139,39 @@ public class SoundTriggerModule {
             if (model instanceof SoundTrigger.GenericSoundModel) {
                 SoundModel aidlModel = ConversionUtil.api2aidlGenericSoundModel(
                         (SoundTrigger.GenericSoundModel) model);
-                soundModelHandle[0] = mService.loadModel(aidlModel);
+                try {
+                    soundModelHandle[0] = mService.loadModel(aidlModel);
+                } finally {
+                    // TODO(b/219825762): We should be able to use the entire object in a
+                    //  try-with-resources
+                    //   clause, instead of having to explicitly close internal fields.
+                    if (aidlModel.data != null) {
+                        try {
+                            aidlModel.data.close();
+                        } catch (IOException e) {
+                            Log.e(TAG, "Failed to close file", e);
+                        }
+                    }
+                }
                 return SoundTrigger.STATUS_OK;
             }
             if (model instanceof SoundTrigger.KeyphraseSoundModel) {
                 PhraseSoundModel aidlModel = ConversionUtil.api2aidlPhraseSoundModel(
                         (SoundTrigger.KeyphraseSoundModel) model);
-                soundModelHandle[0] = mService.loadPhraseModel(aidlModel);
+                try {
+                    soundModelHandle[0] = mService.loadPhraseModel(aidlModel);
+                } finally {
+                    // TODO(b/219825762): We should be able to use the entire object in a
+                    //  try-with-resources
+                    //   clause, instead of having to explicitly close internal fields.
+                    if (aidlModel.common.data != null) {
+                        try {
+                            aidlModel.common.data.close();
+                        } catch (IOException e) {
+                            Log.e(TAG, "Failed to close file", e);
+                        }
+                    }
+                }
                 return SoundTrigger.STATUS_OK;
             }
             return SoundTrigger.STATUS_BAD_VALUE;

services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerHw2Compat.java

@@ -29,7 +29,9 @@ import android.os.IBinder;
 import android.os.IHwBinder;
 import android.os.RemoteException;
 import android.system.OsConstants;
+import android.util.Log;
 
+import java.io.IOException;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Objects;
@@ -54,6 +56,8 @@ import java.util.concurrent.atomic.AtomicReference;
  * </ul>
  */
 final class SoundTriggerHw2Compat implements ISoundTriggerHal {
+    private static final String TAG = "SoundTriggerHw2Compat";
+
     private final @NonNull Runnable mRebootRunnable;
     private final @NonNull IHwBinder mBinder;
     private @NonNull android.hardware.soundtrigger.V2_0.ISoundTriggerHw mUnderlying_2_0;
@@ -226,6 +230,16 @@ final class SoundTriggerHw2Compat implements ISoundTriggerHal {
             return handle.get();
         } catch (RemoteException e) {
             throw e.rethrowAsRuntimeException();
+        } finally {
+            // TODO(b/219825762): We should be able to use the entire object in a try-with-resources
+            //   clause, instead of having to explicitly close internal fields.
+            if (hidlModel.data != null) {
+                try {
+                    hidlModel.data.close();
+                } catch (IOException e) {
+                    Log.e(TAG, "Failed to close file", e);
+                }
+            }
         }
     }
 
@@ -252,6 +266,16 @@ final class SoundTriggerHw2Compat implements ISoundTriggerHal {
             return handle.get();
         } catch (RemoteException e) {
             throw e.rethrowAsRuntimeException();
+        } finally {
+            // TODO(b/219825762): We should be able to use the entire object in a try-with-resources
+            //   clause, instead of having to explicitly close internal fields.
+            if (hidlModel.common.data != null) {
+                try {
+                    hidlModel.common.data.close();
+                } catch (IOException e) {
+                    Log.e(TAG, "Failed to close file", e);
+                }
+            }
         }
     }
 

services/tests/servicestests/src/com/android/server/soundtrigger_middleware/SoundHw2CompatTest.java

@@ -31,7 +31,6 @@ import static org.mockito.Mockito.atMost;
 import static org.mockito.Mockito.clearInvocations;
 import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.mock;
-import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.spy;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.verifyNoMoreInteractions;
@@ -49,7 +48,6 @@ import android.os.IBinder;
 import android.os.IHwBinder;
 import android.os.IHwInterface;
 import android.os.RemoteException;
-import android.system.OsConstants;
 
 import org.junit.After;
 import org.junit.Before;
@@ -60,6 +58,7 @@ import org.mockito.ArgumentCaptor;
 
 import java.util.LinkedList;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicReference;
 
 @RunWith(Parameterized.class)
 public class SoundHw2CompatTest {
@@ -240,14 +239,15 @@ public class SoundHw2CompatTest {
                 (android.hardware.soundtrigger.V2_1.ISoundTriggerHw) mHalDriver;
 
         final int handle = 29;
-        ArgumentCaptor<android.hardware.soundtrigger.V2_1.ISoundTriggerHw.SoundModel> modelCaptor =
+        AtomicReference<android.hardware.soundtrigger.V2_1.ISoundTriggerHw.SoundModel> model =
-                ArgumentCaptor.forClass(
+                new AtomicReference<>();
-                        android.hardware.soundtrigger.V2_1.ISoundTriggerHw.SoundModel.class);
         ArgumentCaptor<android.hardware.soundtrigger.V2_1.ISoundTriggerHwCallback> callbackCaptor =
                 ArgumentCaptor.forClass(
                         android.hardware.soundtrigger.V2_1.ISoundTriggerHwCallback.class);
 
         doAnswer(invocation -> {
+            // We need to dup the model, as it gets invalidated after the call returns.
+            model.set(TestUtil.dupModel_2_1(invocation.getArgument(0)));
             android.hardware.soundtrigger.V2_1.ISoundTriggerHw.loadSoundModel_2_1Callback
                     resultCallback = invocation.getArgument(3);
 
@@ -259,10 +259,9 @@ public class SoundHw2CompatTest {
         assertEquals(handle,
                 mCanonical.loadSoundModel(TestUtil.createGenericSoundModel(), canonicalCallback));
 
-        verify(driver_2_1).loadSoundModel_2_1(modelCaptor.capture(), callbackCaptor.capture(),
+        verify(driver_2_1).loadSoundModel_2_1(any(), callbackCaptor.capture(), anyInt(), any());
-                anyInt(), any());
 
-        TestUtil.validateGenericSoundModel_2_1(modelCaptor.getValue());
+        TestUtil.validateGenericSoundModel_2_1(model.get());
         validateCallback_2_1(callbackCaptor.getValue(), canonicalCallback);
         return handle;
     }
@@ -355,14 +354,16 @@ public class SoundHw2CompatTest {
                 (android.hardware.soundtrigger.V2_1.ISoundTriggerHw) mHalDriver;
 
         final int handle = 29;
-        ArgumentCaptor<android.hardware.soundtrigger.V2_1.ISoundTriggerHw.PhraseSoundModel>
+        AtomicReference<android.hardware.soundtrigger.V2_1.ISoundTriggerHw.PhraseSoundModel> model =
-                modelCaptor = ArgumentCaptor.forClass(
+                new AtomicReference<>();
-                android.hardware.soundtrigger.V2_1.ISoundTriggerHw.PhraseSoundModel.class);
         ArgumentCaptor<android.hardware.soundtrigger.V2_1.ISoundTriggerHwCallback> callbackCaptor =
                 ArgumentCaptor.forClass(
                         android.hardware.soundtrigger.V2_1.ISoundTriggerHwCallback.class);
 
         doAnswer(invocation -> {
+            // We need to dup the model, as it gets invalidated after the call returns.
+            model.set(TestUtil.dupPhraseModel_2_1(invocation.getArgument(0)));
+
             android.hardware.soundtrigger.V2_1.ISoundTriggerHw.loadPhraseSoundModel_2_1Callback
                     resultCallback = invocation.getArgument(3);
 
@@ -374,10 +375,10 @@ public class SoundHw2CompatTest {
         assertEquals(handle, mCanonical.loadPhraseSoundModel(TestUtil.createPhraseSoundModel(),
                 canonicalCallback));
 
-        verify(driver_2_1).loadPhraseSoundModel_2_1(modelCaptor.capture(), callbackCaptor.capture(),
+        verify(driver_2_1).loadPhraseSoundModel_2_1(any(), callbackCaptor.capture(), anyInt(),
-                anyInt(), any());
+                any());
 
-        TestUtil.validatePhraseSoundModel_2_1(modelCaptor.getValue());
+        TestUtil.validatePhraseSoundModel_2_1(model.get());
         validateCallback_2_1(callbackCaptor.getValue(), canonicalCallback);
         return handle;
     }

services/tests/servicestests/src/com/android/server/soundtrigger_middleware/TestUtil.java

@@ -47,6 +47,7 @@ import android.os.HidlMemoryUtil;
 import android.os.ParcelFileDescriptor;
 import android.os.SharedMemory;
 
+import java.io.IOException;
 import java.nio.ByteBuffer;
 import java.util.List;
 
@@ -82,6 +83,19 @@ class TestUtil {
                 HidlMemoryUtil.hidlMemoryToByteArray(model.data));
     }
 
+    static android.hardware.soundtrigger.V2_1.ISoundTriggerHw.SoundModel dupModel_2_1(
+            android.hardware.soundtrigger.V2_1.ISoundTriggerHw.SoundModel model) {
+        android.hardware.soundtrigger.V2_1.ISoundTriggerHw.SoundModel dup =
+                new android.hardware.soundtrigger.V2_1.ISoundTriggerHw.SoundModel();
+        dup.header = model.header;
+        try {
+            dup.data = model.data.dup();
+        } catch (IOException e) {
+            throw new RuntimeException(e);
+        }
+        return dup;
+    }
+
     private static void validateSoundModel_2_0(
             android.hardware.soundtrigger.V2_0.ISoundTriggerHw.SoundModel model, int type) {
         assertEquals(type, model.type);
@@ -121,6 +135,15 @@ class TestUtil {
         validatePhrases_2_0(model.phrases);
     }
 
+    static android.hardware.soundtrigger.V2_1.ISoundTriggerHw.PhraseSoundModel dupPhraseModel_2_1(
+            android.hardware.soundtrigger.V2_1.ISoundTriggerHw.PhraseSoundModel model) {
+        android.hardware.soundtrigger.V2_1.ISoundTriggerHw.PhraseSoundModel dup =
+                new android.hardware.soundtrigger.V2_1.ISoundTriggerHw.PhraseSoundModel();
+        dup.common = dupModel_2_1(model.common);
+        dup.phrases = model.phrases;
+        return dup;
+    }
+
     static void validatePhraseSoundModel_2_0(
             android.hardware.soundtrigger.V2_0.ISoundTriggerHw.PhraseSoundModel model) {
         validateSoundModel_2_0(model.common,
@@ -190,31 +213,27 @@ class TestUtil {
         properties.maxKeyPhrases = 567;
         properties.maxUsers = 678;
         properties.recognitionModes =
-                RecognitionMode.VOICE_TRIGGER
+                RecognitionMode.VOICE_TRIGGER | RecognitionMode.USER_IDENTIFICATION
-                        | RecognitionMode.USER_IDENTIFICATION
+                        | RecognitionMode.USER_AUTHENTICATION | RecognitionMode.GENERIC_TRIGGER;
-                        | RecognitionMode.USER_AUTHENTICATION
-                        | RecognitionMode.GENERIC_TRIGGER;
         properties.captureTransition = true;
         properties.maxBufferMs = 321;
         properties.concurrentCapture = supportConcurrentCapture;
         properties.triggerInEvent = true;
         properties.powerConsumptionMw = 432;
         properties.supportedModelArch = "supportedModelArch";
-        properties.audioCapabilities = AudioCapabilities.ECHO_CANCELLATION
+        properties.audioCapabilities =
-                | AudioCapabilities.NOISE_SUPPRESSION;
+                AudioCapabilities.ECHO_CANCELLATION | AudioCapabilities.NOISE_SUPPRESSION;
         return properties;
     }
 
-    static void validateDefaultProperties(Properties properties,
+    static void validateDefaultProperties(Properties properties, boolean supportConcurrentCapture) {
-            boolean supportConcurrentCapture) {
         validateDefaultProperties(properties, supportConcurrentCapture,
                 AudioCapabilities.ECHO_CANCELLATION | AudioCapabilities.NOISE_SUPPRESSION,
                 "supportedModelArch");
     }
 
-    static void validateDefaultProperties(Properties properties,
+    static void validateDefaultProperties(Properties properties, boolean supportConcurrentCapture,
-            boolean supportConcurrentCapture, @AudioCapabilities int audioCapabilities,
+            @AudioCapabilities int audioCapabilities, @NonNull String supportedModelArch) {
-            @NonNull String supportedModelArch) {
         assertEquals("implementor", properties.implementor);
         assertEquals("description", properties.description);
         assertEquals(123, properties.version);
@@ -222,10 +241,9 @@ class TestUtil {
         assertEquals(456, properties.maxSoundModels);
         assertEquals(567, properties.maxKeyPhrases);
         assertEquals(678, properties.maxUsers);
-        assertEquals(RecognitionMode.GENERIC_TRIGGER
+        assertEquals(RecognitionMode.GENERIC_TRIGGER | RecognitionMode.USER_AUTHENTICATION
-                | RecognitionMode.USER_AUTHENTICATION
+                        | RecognitionMode.USER_IDENTIFICATION | RecognitionMode.VOICE_TRIGGER,
-                | RecognitionMode.USER_IDENTIFICATION
+                properties.recognitionModes);
-                | RecognitionMode.VOICE_TRIGGER, properties.recognitionModes);
         assertTrue(properties.captureTransition);
         assertEquals(321, properties.maxBufferMs);
         assertEquals(supportConcurrentCapture, properties.concurrentCapture);
@@ -246,8 +264,8 @@ class TestUtil {
         config.phraseRecognitionExtras[0].levels[0].userId = 234;
         config.phraseRecognitionExtras[0].levels[0].levelPercent = 34;
         config.data = new byte[]{5, 4, 3, 2, 1};
-        config.audioCapabilities = AudioCapabilities.ECHO_CANCELLATION
+        config.audioCapabilities =
-                | AudioCapabilities.NOISE_SUPPRESSION;
+                AudioCapabilities.ECHO_CANCELLATION | AudioCapabilities.NOISE_SUPPRESSION;
         return config;
     }
 
@@ -295,13 +313,12 @@ class TestUtil {
             int captureHandle) {
         validateRecognitionConfig_2_1(config.base, captureDevice, captureHandle);
 
-        assertEquals(AudioCapabilities.ECHO_CANCELLATION
+        assertEquals(AudioCapabilities.ECHO_CANCELLATION | AudioCapabilities.NOISE_SUPPRESSION,
-                | AudioCapabilities.NOISE_SUPPRESSION, config.audioCapabilities);
+                config.audioCapabilities);
     }
 
     static android.hardware.soundtrigger.V2_0.ISoundTriggerHwCallback.RecognitionEvent createRecognitionEvent_2_0(
-            int hwHandle,
+            int hwHandle, int status) {
-            int status) {
         android.hardware.soundtrigger.V2_0.ISoundTriggerHwCallback.RecognitionEvent halEvent =
                 new android.hardware.soundtrigger.V2_0.ISoundTriggerHwCallback.RecognitionEvent();
         halEvent.status = status;
@@ -351,8 +368,7 @@ class TestUtil {
         return event;
     }
 
-    static ISoundTriggerHwCallback.RecognitionEvent createRecognitionEvent_2_1(
+    static ISoundTriggerHwCallback.RecognitionEvent createRecognitionEvent_2_1(int hwHandle,
-            int hwHandle,
             int status) {
         ISoundTriggerHwCallback.RecognitionEvent halEvent =
                 new ISoundTriggerHwCallback.RecognitionEvent();
@@ -386,8 +402,7 @@ class TestUtil {
         PhraseRecognitionExtra extra = new PhraseRecognitionExtra();
         extra.id = 123;
         extra.confidenceLevel = 52;
-        extra.recognitionModes = RecognitionMode.VOICE_TRIGGER
+        extra.recognitionModes = RecognitionMode.VOICE_TRIGGER | RecognitionMode.GENERIC_TRIGGER;
-                | RecognitionMode.GENERIC_TRIGGER;
         ConfidenceLevel level = new ConfidenceLevel();
         level.userId = 31;
         level.levelPercent = 43;
@@ -396,8 +411,8 @@ class TestUtil {
         return event;
     }
 
-    static android.hardware.soundtrigger.V2_0.ISoundTriggerHwCallback.PhraseRecognitionEvent
+    static android.hardware.soundtrigger.V2_0.ISoundTriggerHwCallback.PhraseRecognitionEvent createPhraseRecognitionEvent_2_0(
-    createPhraseRecognitionEvent_2_0(int hwHandle, int status) {
+            int hwHandle, int status) {
         android.hardware.soundtrigger.V2_0.ISoundTriggerHwCallback.PhraseRecognitionEvent halEvent =
                 new android.hardware.soundtrigger.V2_0.ISoundTriggerHwCallback.PhraseRecognitionEvent();
         halEvent.common = createRecognitionEvent_2_0(hwHandle, status);