Explicitly close sound model file descriptors
This is a workaround for the fact that currently, relying on the GC to
release those resources is unreliable and on the other hand having an
explicit close() that isn't tightly coupled with the internal details
of the parcelable is not possible (filed a bug).
Test: Manual verification of soundtrigger operation.
Test: atest com.android.server.soundtrigger_middleware.SoundHw2CompatTest
Fixes: 219411458
Change-Id: I43bd43ca80eadf97f0254678b0e817fc6f8a06b2
Merged-In: I43bd43ca80eadf97f0254678b0e817fc6f8a06b2
(cherry picked from commit 578eadd1b4227c79972eb87a5856a7f6c20fb4ef)
diff --git a/core/java/android/hardware/soundtrigger/SoundTriggerModule.java b/core/java/android/hardware/soundtrigger/SoundTriggerModule.java
index bf4b514..eed92c1 100644
--- a/core/java/android/hardware/soundtrigger/SoundTriggerModule.java
+++ b/core/java/android/hardware/soundtrigger/SoundTriggerModule.java
@@ -37,6 +37,8 @@
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 @@
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;
diff --git a/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerHw2Compat.java b/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerHw2Compat.java
index c638201..9bbae4b 100644
--- a/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerHw2Compat.java
+++ b/services/core/java/com/android/server/soundtrigger_middleware/SoundTriggerHw2Compat.java
@@ -29,7 +29,9 @@
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 @@
* </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 @@
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 @@
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);
+ }
+ }
}
}
diff --git a/services/tests/servicestests/src/com/android/server/soundtrigger_middleware/SoundHw2CompatTest.java b/services/tests/servicestests/src/com/android/server/soundtrigger_middleware/SoundHw2CompatTest.java
index 16cfd13..6bdd88c 100644
--- a/services/tests/servicestests/src/com/android/server/soundtrigger_middleware/SoundHw2CompatTest.java
+++ b/services/tests/servicestests/src/com/android/server/soundtrigger_middleware/SoundHw2CompatTest.java
@@ -31,7 +31,6 @@
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.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 java.util.LinkedList;
import java.util.List;
+import java.util.concurrent.atomic.AtomicReference;
@RunWith(Parameterized.class)
public class SoundHw2CompatTest {
@@ -240,14 +239,15 @@
(android.hardware.soundtrigger.V2_1.ISoundTriggerHw) mHalDriver;
final int handle = 29;
- ArgumentCaptor<android.hardware.soundtrigger.V2_1.ISoundTriggerHw.SoundModel> modelCaptor =
- ArgumentCaptor.forClass(
- android.hardware.soundtrigger.V2_1.ISoundTriggerHw.SoundModel.class);
+ AtomicReference<android.hardware.soundtrigger.V2_1.ISoundTriggerHw.SoundModel> model =
+ new AtomicReference<>();
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 @@
assertEquals(handle,
mCanonical.loadSoundModel(TestUtil.createGenericSoundModel(), canonicalCallback));
- verify(driver_2_1).loadSoundModel_2_1(modelCaptor.capture(), callbackCaptor.capture(),
- anyInt(), any());
+ verify(driver_2_1).loadSoundModel_2_1(any(), callbackCaptor.capture(), 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 @@
(android.hardware.soundtrigger.V2_1.ISoundTriggerHw) mHalDriver;
final int handle = 29;
- ArgumentCaptor<android.hardware.soundtrigger.V2_1.ISoundTriggerHw.PhraseSoundModel>
- modelCaptor = ArgumentCaptor.forClass(
- android.hardware.soundtrigger.V2_1.ISoundTriggerHw.PhraseSoundModel.class);
+ AtomicReference<android.hardware.soundtrigger.V2_1.ISoundTriggerHw.PhraseSoundModel> model =
+ new AtomicReference<>();
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 @@
assertEquals(handle, mCanonical.loadPhraseSoundModel(TestUtil.createPhraseSoundModel(),
canonicalCallback));
- verify(driver_2_1).loadPhraseSoundModel_2_1(modelCaptor.capture(), callbackCaptor.capture(),
- anyInt(), any());
+ verify(driver_2_1).loadPhraseSoundModel_2_1(any(), callbackCaptor.capture(), anyInt(),
+ any());
- TestUtil.validatePhraseSoundModel_2_1(modelCaptor.getValue());
+ TestUtil.validatePhraseSoundModel_2_1(model.get());
validateCallback_2_1(callbackCaptor.getValue(), canonicalCallback);
return handle;
}
diff --git a/services/tests/servicestests/src/com/android/server/soundtrigger_middleware/TestUtil.java b/services/tests/servicestests/src/com/android/server/soundtrigger_middleware/TestUtil.java
index e687a2a..30b4a59 100644
--- a/services/tests/servicestests/src/com/android/server/soundtrigger_middleware/TestUtil.java
+++ b/services/tests/servicestests/src/com/android/server/soundtrigger_middleware/TestUtil.java
@@ -47,6 +47,7 @@
import android.os.ParcelFileDescriptor;
import android.os.SharedMemory;
+import java.io.IOException;
import java.nio.ByteBuffer;
import java.util.List;
@@ -82,6 +83,19 @@
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 @@
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 @@
properties.maxKeyPhrases = 567;
properties.maxUsers = 678;
properties.recognitionModes =
- RecognitionMode.VOICE_TRIGGER
- | RecognitionMode.USER_IDENTIFICATION
- | RecognitionMode.USER_AUTHENTICATION
- | RecognitionMode.GENERIC_TRIGGER;
+ RecognitionMode.VOICE_TRIGGER | RecognitionMode.USER_IDENTIFICATION
+ | 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
- | AudioCapabilities.NOISE_SUPPRESSION;
+ properties.audioCapabilities =
+ AudioCapabilities.ECHO_CANCELLATION | AudioCapabilities.NOISE_SUPPRESSION;
return properties;
}
- static void validateDefaultProperties(Properties properties,
- boolean supportConcurrentCapture) {
+ static void validateDefaultProperties(Properties properties, boolean supportConcurrentCapture) {
validateDefaultProperties(properties, supportConcurrentCapture,
AudioCapabilities.ECHO_CANCELLATION | AudioCapabilities.NOISE_SUPPRESSION,
"supportedModelArch");
}
- static void validateDefaultProperties(Properties properties,
- boolean supportConcurrentCapture, @AudioCapabilities int audioCapabilities,
- @NonNull String supportedModelArch) {
+ static void validateDefaultProperties(Properties properties, boolean supportConcurrentCapture,
+ @AudioCapabilities int audioCapabilities, @NonNull String supportedModelArch) {
assertEquals("implementor", properties.implementor);
assertEquals("description", properties.description);
assertEquals(123, properties.version);
@@ -222,10 +241,9 @@
assertEquals(456, properties.maxSoundModels);
assertEquals(567, properties.maxKeyPhrases);
assertEquals(678, properties.maxUsers);
- assertEquals(RecognitionMode.GENERIC_TRIGGER
- | RecognitionMode.USER_AUTHENTICATION
- | RecognitionMode.USER_IDENTIFICATION
- | RecognitionMode.VOICE_TRIGGER, properties.recognitionModes);
+ assertEquals(RecognitionMode.GENERIC_TRIGGER | RecognitionMode.USER_AUTHENTICATION
+ | RecognitionMode.USER_IDENTIFICATION | RecognitionMode.VOICE_TRIGGER,
+ properties.recognitionModes);
assertTrue(properties.captureTransition);
assertEquals(321, properties.maxBufferMs);
assertEquals(supportConcurrentCapture, properties.concurrentCapture);
@@ -246,8 +264,8 @@
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
- | AudioCapabilities.NOISE_SUPPRESSION;
+ config.audioCapabilities =
+ AudioCapabilities.ECHO_CANCELLATION | AudioCapabilities.NOISE_SUPPRESSION;
return config;
}
@@ -295,13 +313,12 @@
int captureHandle) {
validateRecognitionConfig_2_1(config.base, captureDevice, captureHandle);
- assertEquals(AudioCapabilities.ECHO_CANCELLATION
- | AudioCapabilities.NOISE_SUPPRESSION, config.audioCapabilities);
+ assertEquals(AudioCapabilities.ECHO_CANCELLATION | AudioCapabilities.NOISE_SUPPRESSION,
+ config.audioCapabilities);
}
static android.hardware.soundtrigger.V2_0.ISoundTriggerHwCallback.RecognitionEvent createRecognitionEvent_2_0(
- int hwHandle,
- int status) {
+ int hwHandle, 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 @@
return event;
}
- static ISoundTriggerHwCallback.RecognitionEvent createRecognitionEvent_2_1(
- int hwHandle,
+ static ISoundTriggerHwCallback.RecognitionEvent createRecognitionEvent_2_1(int hwHandle,
int status) {
ISoundTriggerHwCallback.RecognitionEvent halEvent =
new ISoundTriggerHwCallback.RecognitionEvent();
@@ -386,8 +402,7 @@
PhraseRecognitionExtra extra = new PhraseRecognitionExtra();
extra.id = 123;
extra.confidenceLevel = 52;
- extra.recognitionModes = RecognitionMode.VOICE_TRIGGER
- | RecognitionMode.GENERIC_TRIGGER;
+ extra.recognitionModes = RecognitionMode.VOICE_TRIGGER | RecognitionMode.GENERIC_TRIGGER;
ConfidenceLevel level = new ConfidenceLevel();
level.userId = 31;
level.levelPercent = 43;
@@ -396,8 +411,8 @@
return event;
}
- static android.hardware.soundtrigger.V2_0.ISoundTriggerHwCallback.PhraseRecognitionEvent
- createPhraseRecognitionEvent_2_0(int hwHandle, int status) {
+ static android.hardware.soundtrigger.V2_0.ISoundTriggerHwCallback.PhraseRecognitionEvent createPhraseRecognitionEvent_2_0(
+ 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);