Merge "Clean up on client death in SoundTriggerService"

This commit is contained in:
Ytai Ben-tsvi
2020-11-11 21:31:39 +00:00
committed by Android (Google) Code Review
8 changed files with 98 additions and 20 deletions

View File

@@ -40,7 +40,9 @@ import android.hardware.soundtrigger.SoundTrigger.RecognitionConfig;
import android.media.AudioFormat;
import android.media.permission.Identity;
import android.os.AsyncTask;
import android.os.Binder;
import android.os.Handler;
import android.os.IBinder;
import android.os.Message;
import android.os.RemoteException;
import android.util.Slog;
@@ -246,6 +248,7 @@ public class AlwaysOnHotwordDetector {
private final Callback mExternalCallback;
private final Object mLock = new Object();
private final Handler mHandler;
private final IBinder mBinder = new Binder();
private int mAvailability = STATE_NOT_READY;
@@ -450,7 +453,7 @@ public class AlwaysOnHotwordDetector {
Identity identity = new Identity();
identity.packageName = ActivityThread.currentOpPackageName();
mSoundTriggerSession = mModelManagementService.createSoundTriggerSessionAsOriginator(
identity);
identity, mBinder);
} catch (RemoteException e) {
throw e.rethrowAsRuntimeException();
}

View File

@@ -38,8 +38,12 @@ interface ISoundTriggerService {
*
* It is good practice to clear the binder calling identity prior to calling this, in case the
* caller is ever in the same process as the callee.
*
* The binder object being passed is used by the server to keep track of client death, in order
* to clean-up whenever that happens.
*/
ISoundTriggerSession attachAsOriginator(in Identity originatorIdentity);
ISoundTriggerSession attachAsOriginator(in Identity originatorIdentity,
IBinder client);
/**
* Creates a new session.
@@ -54,7 +58,11 @@ interface ISoundTriggerService {
*
* It is good practice to clear the binder calling identity prior to calling this, in case the
* caller is ever in the same process as the callee.
*
* The binder object being passed is used by the server to keep track of client death, in order
* to clean-up whenever that happens.
*/
ISoundTriggerSession attachAsMiddleman(in Identity middlemanIdentity,
in Identity originatorIdentity);
in Identity originatorIdentity,
IBinder client);
}

View File

@@ -216,7 +216,11 @@ interface IVoiceInteractionManagerService {
* Caller must provide an identity, used for permission tracking purposes.
* The uid/pid elements of the identity will be ignored by the server and replaced with the ones
* provided by binder.
*
* The client argument is any binder owned by the client, used for tracking is death and
* cleaning up in this event.
*/
IVoiceInteractionSoundTriggerSession createSoundTriggerSessionAsOriginator(
in Identity originatorIdentity);
in Identity originatorIdentity,
IBinder client);
}

View File

@@ -41,6 +41,7 @@ import android.os.Binder;
import android.os.Build;
import android.os.Bundle;
import android.os.Handler;
import android.os.IBinder;
import android.os.ParcelUuid;
import android.os.RemoteException;
import android.provider.Settings;
@@ -69,6 +70,7 @@ public final class SoundTriggerManager {
private final Context mContext;
private final ISoundTriggerSession mSoundTriggerSession;
private final IBinder mBinderToken = new Binder();
// Stores a mapping from the sound model UUID to the SoundTriggerInstance created by
// the createSoundTriggerDetector() call.
@@ -90,7 +92,8 @@ public final class SoundTriggerManager {
originatorIdentity.packageName = ActivityThread.currentOpPackageName();
try (SafeCloseable ignored = ClearCallingIdentityContext.create()) {
mSoundTriggerSession = soundTriggerService.attachAsOriginator(originatorIdentity);
mSoundTriggerSession = soundTriggerService.attachAsOriginator(originatorIdentity,
mBinderToken);
}
} catch (RemoteException e) {
throw e.rethrowAsRuntimeException();

View File

@@ -1139,6 +1139,25 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener {
}
}
/**
* Stops and unloads all models. This is intended as a clean-up call with the expectation that
* this instance is not used after.
* @hide
*/
public void detach() {
synchronized (mLock) {
for (ModelData model : mModelDataMap.values()) {
forceStopAndUnloadModelLocked(model, null);
}
mModelDataMap.clear();
internalClearGlobalStateLocked();
if (mModule != null) {
mModule.detach();
mModule = null;
}
}
}
/**
* Stops and unloads a sound model, and removes any reference to the model if successful.
*
@@ -1170,7 +1189,7 @@ public class SoundTriggerHelper implements SoundTrigger.StatusListener {
}
if (modelData.isModelStarted()) {
Slog.d(TAG, "Stopping previously started dangling model " + modelData.getHandle());
if (mModule.stopRecognition(modelData.getHandle()) != STATUS_OK) {
if (mModule.stopRecognition(modelData.getHandle()) == STATUS_OK) {
modelData.setStopped();
modelData.setRequested(false);
} else {

View File

@@ -27,6 +27,7 @@ import android.hardware.soundtrigger.SoundTrigger.ModelParamRange;
import android.hardware.soundtrigger.SoundTrigger.ModuleProperties;
import android.hardware.soundtrigger.SoundTrigger.RecognitionConfig;
import android.media.permission.Identity;
import android.os.IBinder;
import com.android.server.voiceinteraction.VoiceInteractionManagerService;
@@ -46,10 +47,12 @@ public interface SoundTriggerInternal {
int STATUS_ERROR = SoundTrigger.STATUS_ERROR;
int STATUS_OK = SoundTrigger.STATUS_OK;
Session attachAsOriginator(@NonNull Identity originatorIdentity);
Session attachAsOriginator(@NonNull Identity originatorIdentity,
@NonNull IBinder client);
Session attachAsMiddleman(@NonNull Identity middlemanIdentity,
@NonNull Identity originatorIdentity);
@NonNull Identity originatorIdentity,
@NonNull IBinder client);
/**
* Dumps service-wide information.

View File

@@ -226,33 +226,45 @@ public class SoundTriggerService extends SystemService {
class SoundTriggerServiceStub extends ISoundTriggerService.Stub {
@Override
public ISoundTriggerSession attachAsOriginator(Identity originatorIdentity) {
public ISoundTriggerSession attachAsOriginator(Identity originatorIdentity,
@NonNull IBinder client) {
try (SafeCloseable ignored = PermissionUtil.establishIdentityDirect(
originatorIdentity)) {
return new SoundTriggerSessionStub(newSoundTriggerHelper());
return new SoundTriggerSessionStub(newSoundTriggerHelper(), client);
}
}
@Override
public ISoundTriggerSession attachAsMiddleman(Identity originatorIdentity,
Identity middlemanIdentity) {
Identity middlemanIdentity,
@NonNull IBinder client) {
try (SafeCloseable ignored = PermissionUtil.establishIdentityIndirect(mContext,
SOUNDTRIGGER_DELEGATE_IDENTITY, middlemanIdentity,
originatorIdentity)) {
return new SoundTriggerSessionStub(newSoundTriggerHelper());
return new SoundTriggerSessionStub(newSoundTriggerHelper(), client);
}
}
}
class SoundTriggerSessionStub extends ISoundTriggerSession.Stub {
private final SoundTriggerHelper mSoundTriggerHelper;
// Used to detect client death.
private final IBinder mClient;
private final TreeMap<UUID, SoundModel> mLoadedModels = new TreeMap<>();
private final Object mCallbacksLock = new Object();
private final TreeMap<UUID, IRecognitionStatusCallback> mCallbacks = new TreeMap<>();
SoundTriggerSessionStub(
SoundTriggerHelper soundTriggerHelper) {
SoundTriggerHelper soundTriggerHelper, @NonNull IBinder client) {
mSoundTriggerHelper = soundTriggerHelper;
mClient = client;
try {
mClient.linkToDeath(() -> {
clientDied();
}, 0);
} catch (RemoteException e) {
Slog.e(TAG, "Failed to register death listener.", e);
}
}
@Override
@@ -790,6 +802,13 @@ public class SoundTriggerService extends SystemService {
}
}
private void clientDied() {
Slog.w(TAG, "Client died, cleaning up session.");
sEventLogger.log(new SoundTriggerLogger.StringEvent(
"Client died, cleaning up session."));
mSoundTriggerHelper.detach();
}
/**
* Local end for a {@link SoundTriggerDetectionService}. Operations are queued up and
* executed when the service connects.
@@ -1457,10 +1476,19 @@ public class SoundTriggerService extends SystemService {
private class SessionImpl implements Session {
private final @NonNull SoundTriggerHelper mSoundTriggerHelper;
private final @NonNull IBinder mClient;
private SessionImpl(
@NonNull SoundTriggerHelper soundTriggerHelper) {
@NonNull SoundTriggerHelper soundTriggerHelper, @NonNull IBinder client) {
mSoundTriggerHelper = soundTriggerHelper;
mClient = client;
try {
mClient.linkToDeath(() -> {
clientDied();
}, 0);
} catch (RemoteException e) {
Slog.e(TAG, "Failed to register death listener.", e);
}
}
@Override
@@ -1507,22 +1535,31 @@ public class SoundTriggerService extends SystemService {
public void dump(FileDescriptor fd, PrintWriter pw, String[] args) {
mSoundTriggerHelper.dump(fd, pw, args);
}
private void clientDied() {
Slog.w(TAG, "Client died, cleaning up session.");
sEventLogger.log(new SoundTriggerLogger.StringEvent(
"Client died, cleaning up session."));
mSoundTriggerHelper.detach();
}
}
@Override
public Session attachAsOriginator(@NonNull Identity originatorIdentity) {
public Session attachAsOriginator(@NonNull Identity originatorIdentity,
@NonNull IBinder client) {
try (SafeCloseable ignored = PermissionUtil.establishIdentityDirect(
originatorIdentity)) {
return new SessionImpl(newSoundTriggerHelper());
return new SessionImpl(newSoundTriggerHelper(), client);
}
}
@Override
public Session attachAsMiddleman(@NonNull Identity middlemanIdentity,
@NonNull Identity originatorIdentity) {
@NonNull Identity originatorIdentity,
@NonNull IBinder client) {
try (SafeCloseable ignored = PermissionUtil.establishIdentityIndirect(mContext,
SOUNDTRIGGER_DELEGATE_IDENTITY, middlemanIdentity, originatorIdentity)) {
return new SessionImpl(newSoundTriggerHelper());
return new SessionImpl(newSoundTriggerHelper(), client);
}
}

View File

@@ -257,12 +257,13 @@ public class VoiceInteractionManagerService extends SystemService {
@Override
public @NonNull IVoiceInteractionSoundTriggerSession createSoundTriggerSessionAsOriginator(
@NonNull Identity originatorIdentity) {
@NonNull Identity originatorIdentity, IBinder client) {
Objects.requireNonNull(originatorIdentity);
try (SafeCloseable ignored = PermissionUtil.establishIdentityDirect(
originatorIdentity)) {
SoundTriggerSession session = new SoundTriggerSession(
mSoundTriggerInternal.attachAsOriginator(IdentityContext.getNonNull()));
mSoundTriggerInternal.attachAsOriginator(IdentityContext.getNonNull(),
client));
synchronized (mSessions) {
mSessions.add(new WeakReference<>(session));
}