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

Commit dd91ce21 authored by Mikhail Naganov's avatar Mikhail Naganov
Browse files

PatchPanel: Fix crash when tearing down "pass thru" software patch 

The pass thru software patch is normally used with the MSD module.

There is no need to establish a peer reference from PatchRecord
because its I/O code only gets executed by PatchTrack
initiative. In fact, this also means PassthroughPatchRecord
can't be stopped synchronously, so the usual software patch tear
down logic in PatchPanel leads to a null pointer dereference
because it races with PatchTrack I/O activity on a separate
thread.

Fix by skipping the sp<> setup / clear logic in PatchPanel
for PassthroughPatchRecord.

Bug: 147599144
Test: force teardown of MSD patch, check logcat for crash
Change-Id: I4a2abc16ad705244767b33ea529e1ace2213d19f
parent 3c9b9af1

services/audioflinger/PatchPanel.cpp

+11 −3
Original line number Diff line number Diff line
@@ -490,10 +490,12 @@ status_t AudioFlinger::PatchPanel::Patch::createConnections(PatchPanel *panel) 
    }



    sp<RecordThread::PatchRecord> tempRecordTrack;

    const bool usePassthruPatchRecord =

            (inputFlags & AUDIO_INPUT_FLAG_DIRECT) && (outputFlags & AUDIO_OUTPUT_FLAG_DIRECT);

    const size_t playbackFrameCount = mPlayback.thread()->frameCount();

    const size_t recordFrameCount = mRecord.thread()->frameCount();

    size_t frameCount = 0;

    if ((inputFlags & AUDIO_INPUT_FLAG_DIRECT) && (outputFlags & AUDIO_OUTPUT_FLAG_DIRECT)) {

    if (usePassthruPatchRecord) {

        // PassthruPatchRecord producesBufferOnDemand, so use

        // maximum of playback and record thread framecounts

        frameCount = std::max(playbackFrameCount, recordFrameCount);
@@ -550,8 +552,14 @@ status_t AudioFlinger::PatchPanel::Patch::createConnections(PatchPanel *panel) 
    }



    // tie playback and record tracks together

    mRecord.setTrackAndPeer(tempRecordTrack, tempPatchTrack);

    mPlayback.setTrackAndPeer(tempPatchTrack, tempRecordTrack);

    // In the case of PassthruPatchRecord no I/O activity happens on RecordThread,

    // everything is driven from PlaybackThread. Thus AudioBufferProvider methods

    // of PassthruPatchRecord can only be called if the corresponding PatchTrack

    // is alive. There is no need to hold a reference, and there is no need

    // to clear it. In fact, since playback stopping is asynchronous, there is

    // no proper time when clearing could be done.

    mRecord.setTrackAndPeer(tempRecordTrack, tempPatchTrack, !usePassthruPatchRecord);

    mPlayback.setTrackAndPeer(tempPatchTrack, tempRecordTrack, true /*holdReference*/);



    // start capture and playback

    mRecord.track()->start(AudioSystem::SYNC_EVENT_NONE, AUDIO_SESSION_NONE);

services/audioflinger/PatchPanel.h

+6 −3
Original line number Diff line number Diff line
@@ -123,18 +123,20 @@ private: 
            mCloseThread = closeThread;

        }

        template <typename T>

        void setTrackAndPeer(const sp<TrackType>& track, const sp<T> &peer) {

        void setTrackAndPeer(const sp<TrackType>& track, const sp<T> &peer, bool holdReference) {

            mTrack = track;

            mThread->addPatchTrack(mTrack);

            mTrack->setPeerProxy(peer, true /* holdReference */);

            mTrack->setPeerProxy(peer, holdReference);

            mClearPeerProxy = holdReference;

        }

        void clearTrackPeer() { if (mTrack) mTrack->clearPeerProxy(); }

        void clearTrackPeer() { if (mClearPeerProxy && mTrack) mTrack->clearPeerProxy(); }

        void stopTrack() { if (mTrack) mTrack->stop(); }



        void swap(Endpoint &other) noexcept {

            using std::swap;

            swap(mThread, other.mThread);

            swap(mCloseThread, other.mCloseThread);

            swap(mClearPeerProxy, other.mClearPeerProxy);

            swap(mHandle, other.mHandle);

            swap(mTrack, other.mTrack);

        }
@@ -146,6 +148,7 @@ private: 
    private:

        sp<ThreadType> mThread;

        bool mCloseThread = true;

        bool mClearPeerProxy = true;

        audio_patch_handle_t mHandle = AUDIO_PATCH_HANDLE_NONE;

        sp<TrackType> mTrack;

    };