Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ae5fb990 authored by Chris Thornton's avatar Chris Thornton
Browse files

Fix deadlock in SoundTriggerService using intent API. 

The callback of the SoundTriggerService using the intent API used to try
and grab the same lock that other calls to the STS were using when they
accessed the SoundTriggerHelper.

This happens because most functions in the STH grab the STH's lock,
including the one that handles the recognition events. The recognition
event callback in the STS would then try to grab the STS lock, while
implicitly holding the STH one.

However, a concurrent call to the STS from outside could first grab the
STS lock, then call into the STH which may need the STH lock, resulting
in a deadlock.

By removing the requirement that the STS callback grab the main STS
lock, this condition is avoided.

Bug: 70346433
Test: On device
Change-Id: I44571fba786a82a17423d45f503be9537b476a01
parent 153daa89

services/voiceinteraction/java/com/android/server/soundtrigger/SoundTriggerService.java

+27 −13
Original line number Original line Diff line number Diff line
@@ -66,6 +66,7 @@ public class SoundTriggerService extends SystemService { 
    private SoundTriggerDbHelper mDbHelper;

    private SoundTriggerDbHelper mDbHelper;

    private SoundTriggerHelper mSoundTriggerHelper;

    private SoundTriggerHelper mSoundTriggerHelper;

    private final TreeMap<UUID, SoundModel> mLoadedModels;

    private final TreeMap<UUID, SoundModel> mLoadedModels;

    private Object mCallbacksLock;

    private final TreeMap<UUID, LocalSoundTriggerRecognitionStatusCallback> mIntentCallbacks;

    private final TreeMap<UUID, LocalSoundTriggerRecognitionStatusCallback> mIntentCallbacks;

    private PowerManager.WakeLock mWakelock;

    private PowerManager.WakeLock mWakelock;
@@ -75,6 +76,7 @@ public class SoundTriggerService extends SystemService { 
        mServiceStub = new SoundTriggerServiceStub();

        mServiceStub = new SoundTriggerServiceStub();

        mLocalSoundTriggerService = new LocalSoundTriggerService(context);

        mLocalSoundTriggerService = new LocalSoundTriggerService(context);

        mLoadedModels = new TreeMap<UUID, SoundModel>();

        mLoadedModels = new TreeMap<UUID, SoundModel>();

        mCallbacksLock = new Object();

        mIntentCallbacks = new TreeMap<UUID, LocalSoundTriggerRecognitionStatusCallback>();

        mIntentCallbacks = new TreeMap<UUID, LocalSoundTriggerRecognitionStatusCallback>();

        mLock = new Object();

        mLock = new Object();

    }

    }
@@ -211,8 +213,10 @@ public class SoundTriggerService extends SystemService { 
                // don't know if the other model is loaded.

                // don't know if the other model is loaded.

                if (oldModel != null && !oldModel.equals(soundModel)) {

                if (oldModel != null && !oldModel.equals(soundModel)) {

                    mSoundTriggerHelper.unloadGenericSoundModel(soundModel.uuid);

                    mSoundTriggerHelper.unloadGenericSoundModel(soundModel.uuid);

                    synchronized (mCallbacksLock) {

                        mIntentCallbacks.remove(soundModel.uuid);

                        mIntentCallbacks.remove(soundModel.uuid);

                    }

                    }

                }

                mLoadedModels.put(soundModel.uuid, soundModel);

                mLoadedModels.put(soundModel.uuid, soundModel);

            }

            }

            return STATUS_OK;

            return STATUS_OK;
@@ -240,8 +244,10 @@ public class SoundTriggerService extends SystemService { 
                // don't know if the other model is loaded.

                // don't know if the other model is loaded.

                if (oldModel != null && !oldModel.equals(soundModel)) {

                if (oldModel != null && !oldModel.equals(soundModel)) {

                    mSoundTriggerHelper.unloadKeyphraseSoundModel(soundModel.keyphrases[0].id);

                    mSoundTriggerHelper.unloadKeyphraseSoundModel(soundModel.keyphrases[0].id);

                    synchronized (mCallbacksLock) {

                        mIntentCallbacks.remove(soundModel.uuid);

                        mIntentCallbacks.remove(soundModel.uuid);

                    }

                    }

                }

                mLoadedModels.put(soundModel.uuid, soundModel);

                mLoadedModels.put(soundModel.uuid, soundModel);

            }

            }

            return STATUS_OK;

            return STATUS_OK;
@@ -262,8 +268,10 @@ public class SoundTriggerService extends SystemService { 
                    Slog.e(TAG, soundModelId + " is not loaded");

                    Slog.e(TAG, soundModelId + " is not loaded");

                    return STATUS_ERROR;

                    return STATUS_ERROR;

                }

                }

                LocalSoundTriggerRecognitionStatusCallback callback = mIntentCallbacks.get(

                LocalSoundTriggerRecognitionStatusCallback callback = null;

                        soundModelId.getUuid());

                synchronized (mCallbacksLock) {

                    callback = mIntentCallbacks.get(soundModelId.getUuid());

                }

                if (callback != null) {

                if (callback != null) {

                    Slog.e(TAG, soundModelId + " is already running");

                    Slog.e(TAG, soundModelId + " is already running");

                    return STATUS_ERROR;

                    return STATUS_ERROR;
@@ -291,8 +299,10 @@ public class SoundTriggerService extends SystemService { 
                    Slog.e(TAG, "Failed to start model: " + ret);

                    Slog.e(TAG, "Failed to start model: " + ret);

                    return ret;

                    return ret;

                }

                }

                synchronized (mCallbacksLock) {

                    mIntentCallbacks.put(soundModelId.getUuid(), callback);

                    mIntentCallbacks.put(soundModelId.getUuid(), callback);

                }

                }

            }

            return STATUS_OK;

            return STATUS_OK;

        }

        }
@@ -310,8 +320,10 @@ public class SoundTriggerService extends SystemService { 
                    Slog.e(TAG, soundModelId + " is not loaded");

                    Slog.e(TAG, soundModelId + " is not loaded");

                    return STATUS_ERROR;

                    return STATUS_ERROR;

                }

                }

                LocalSoundTriggerRecognitionStatusCallback callback = mIntentCallbacks.get(

                LocalSoundTriggerRecognitionStatusCallback callback = null;

                        soundModelId.getUuid());

                synchronized (mCallbacksLock) {

                     callback = mIntentCallbacks.get(soundModelId.getUuid());

                }

                if (callback == null) {

                if (callback == null) {

                    Slog.e(TAG, soundModelId + " is not running");

                    Slog.e(TAG, soundModelId + " is not running");

                    return STATUS_ERROR;

                    return STATUS_ERROR;
@@ -334,8 +346,10 @@ public class SoundTriggerService extends SystemService { 
                    Slog.e(TAG, "Failed to stop model: " + ret);

                    Slog.e(TAG, "Failed to stop model: " + ret);

                    return ret;

                    return ret;

                }

                }

                synchronized (mCallbacksLock) {

                    mIntentCallbacks.remove(soundModelId.getUuid());

                    mIntentCallbacks.remove(soundModelId.getUuid());

                }

                }

            }

            return STATUS_OK;

            return STATUS_OK;

        }

        }
@@ -379,14 +393,14 @@ public class SoundTriggerService extends SystemService { 
        public boolean isRecognitionActive(ParcelUuid parcelUuid) {

        public boolean isRecognitionActive(ParcelUuid parcelUuid) {

            enforceCallingPermission(Manifest.permission.MANAGE_SOUND_TRIGGER);

            enforceCallingPermission(Manifest.permission.MANAGE_SOUND_TRIGGER);

            if (!isInitialized()) return false;

            if (!isInitialized()) return false;

            synchronized (mLock) {

            synchronized (mCallbacksLock) {

                LocalSoundTriggerRecognitionStatusCallback callback =

                LocalSoundTriggerRecognitionStatusCallback callback =

                        mIntentCallbacks.get(parcelUuid.getUuid());

                        mIntentCallbacks.get(parcelUuid.getUuid());

                if (callback == null) {

                if (callback == null) {

                    return false;

                    return false;

                }

                }

                return mSoundTriggerHelper.isRecognitionRequested(parcelUuid.getUuid());

            }

            }

            return mSoundTriggerHelper.isRecognitionRequested(parcelUuid.getUuid());

        }

        }

    }

    }
@@ -513,7 +527,7 @@ public class SoundTriggerService extends SystemService { 




        private void removeCallback(boolean releaseWakeLock) {

        private void removeCallback(boolean releaseWakeLock) {

            mCallbackIntent = null;

            mCallbackIntent = null;

            synchronized (mLock) {

            synchronized (mCallbacksLock) {

                mIntentCallbacks.remove(mUuid);

                mIntentCallbacks.remove(mUuid);

                if (releaseWakeLock) {

                if (releaseWakeLock) {

                    mWakelock.release();

                    mWakelock.release();
@@ -523,7 +537,7 @@ public class SoundTriggerService extends SystemService { 
    }

    }





    private void grabWakeLock() {

    private void grabWakeLock() {

        synchronized (mLock) {

        synchronized (mCallbacksLock) {

            if (mWakelock == null) {

            if (mWakelock == null) {

                PowerManager pm = ((PowerManager) mContext.getSystemService(Context.POWER_SERVICE));

                PowerManager pm = ((PowerManager) mContext.getSystemService(Context.POWER_SERVICE));

                mWakelock = pm.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, TAG);

                mWakelock = pm.newWakeLock(PowerManager.PARTIAL_WAKE_LOCK, TAG);
@@ -537,7 +551,7 @@ public class SoundTriggerService extends SystemService { 
        public void onSendFinished(PendingIntent pendingIntent, Intent intent, int resultCode,

        public void onSendFinished(PendingIntent pendingIntent, Intent intent, int resultCode,

                String resultData, Bundle resultExtras) {

                String resultData, Bundle resultExtras) {

            // We're only ever invoked when the callback is done, so release the lock.

            // We're only ever invoked when the callback is done, so release the lock.

            synchronized (mLock) {

            synchronized (mCallbacksLock) {

                mWakelock.release();

                mWakelock.release();

            }

            }

        }

        }