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

Commit 6f2d0e6c authored by Atneya Nair's avatar Atneya Nair
Browse files

[soundtrigger] Fix releaseAudioSession deadlock 

Calling into releaseAudioSession holding the STM lock is dangerous, as
the STM lock is waited for by STHAL callbacks (including abort). This
callback fires from the HAL while servicing startRecord calls, which
holds APM/AF locks. releaseAudioSession waits for the APM lock,
causing a cross deadlock.

Although the STHAL callback model in general is prone to these deadlocks
due to upcalls, spot-fix this by pulling the releaseAudioSession call
outside of the ST locks. This is fine, since there are no
post-conditions on this call: it is essentially cleanup/gc (all the ids
involved are monotonically increasing).

Test: manual hotword
Test: manual now playing
Flag: EXEMPT bugfix
Bug: 391620601
Change-Id: I67b8b2ee484be7c7aefb26d9df64689e5e3f5d00
parent 5e659e32

services/voiceinteraction/java/com/android/server/soundtrigger_middleware/SoundTriggerModule.java

+25 −23
Original line number Diff line number Diff line
@@ -257,58 +257,60 @@ class SoundTriggerModule implements IBinder.DeathRecipient, ISoundTriggerHal.Glo 


        @Override

        public int loadModel(@NonNull SoundModel model) {

            synchronized (SoundTriggerModule.this) {

                SoundTriggerMiddlewareImpl.AudioSessionProvider.AudioSession audioSession =

                        mAudioSessionProvider.acquireSession();

            SoundTriggerMiddlewareImpl.AudioSessionProvider.AudioSession audioSession = null;

            try {

                synchronized (SoundTriggerModule.this) {

                    audioSession = mAudioSessionProvider.acquireSession();

                    checkValid();

                    Model loadedModel = new Model();

                    return loadedModel.load(model, audioSession);

                }

            } catch (Exception e) {

                    // We must do this outside the lock, to avoid possible deadlocks with the remote

                    // process that provides the audio sessions, which may also be calling into us.

                try {

                    if (audioSession != null) {

                        mAudioSessionProvider.releaseSession(audioSession.mSessionHandle);

                    }

                } catch (Exception ee) {

                    Slog.e(TAG, "Failed to release session.", ee);

                }

                throw e;

            }

        }

        }



        @Override

        public int loadPhraseModel(@NonNull PhraseSoundModel model) {

            synchronized (SoundTriggerModule.this) {

                SoundTriggerMiddlewareImpl.AudioSessionProvider.AudioSession audioSession =

                        mAudioSessionProvider.acquireSession();

            SoundTriggerMiddlewareImpl.AudioSessionProvider.AudioSession audioSession = null;

            try {

                synchronized (SoundTriggerModule.this) {

                    audioSession = mAudioSessionProvider.acquireSession();

                    checkValid();

                    Model loadedModel = new Model();

                    int result = loadedModel.load(model, audioSession);

                    Slog.d(TAG, String.format("loadPhraseModel()->%d", result));

                    return result;

                }

            } catch (Exception e) {

                    // We must do this outside the lock, to avoid possible deadlocks with the remote

                    // process that provides the audio sessions, which may also be calling into us.

                try {

                    if (audioSession != null) {

                        mAudioSessionProvider.releaseSession(audioSession.mSessionHandle);

                    }

                } catch (Exception ee) {

                    Slog.e(TAG, "Failed to release session.", ee);

                }

                throw e;

            }

        }

        }



        @Override

        public void unloadModel(int modelHandle) {

            int sessionHandle;

            synchronized (SoundTriggerModule.this) {

                checkValid();

                final var session = mLoadedModels.get(modelHandle).getSession();

                sessionHandle = session.mSessionHandle;

                mLoadedModels.remove(modelHandle);

                mAudioSessionProvider.releaseSession(session.mSessionHandle);

            }

            mAudioSessionProvider.releaseSession(sessionHandle);

            // We don't need to post-synchronize on anything once the HAL has finished the unload

            // and dispatched any appropriate callbacks -- since we don't do any state checking

            // onModelUnloaded regardless.