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

Commit 52bc293a authored by Phil Burk's avatar Phil Burk Committed by Automerger Merge Worker
Browse files

aaudio: fix crash from callbacks during close am: 4719048c

Original change: https://googleplex-android-review.googlesource.com/c/platform/frameworks/av/+/12540591

Change-Id: Ic046f39f0327c662362ceed7b090bb2cf91e2e1b
parents 60669b44 4719048c
Loading
Loading
Loading
Loading
+5 −0
Original line number Diff line number Diff line
@@ -1037,6 +1037,11 @@ AAUDIO_API aaudio_result_t AAudioStreamBuilder_delete(AAudioStreamBuilder* buil
 * but still allow queries to the stream to occur from other threads. This often
 * happens if you are monitoring stream progress from a UI thread.
 *
 * NOTE: This function is only fully implemented for MMAP streams,
 * which are low latency streams supported by some devices.
 * On other "Legacy" streams some audio resources will still be in use
 * and some callbacks may still be in process after this call.
 *
 * @param stream reference provided by AAudioStreamBuilder_openStream()
 * @return {@link #AAUDIO_OK} or a negative error.
 */
+2 −4
Original line number Diff line number Diff line
@@ -255,16 +255,14 @@ AAUDIO_API aaudio_result_t AAudioStream_close(AAudioStream* stream) {
    if (audioStream != nullptr) {
        aaudio_stream_id_t id = audioStream->getId();
        ALOGD("%s(s#%u) called ---------------", __func__, id);
        result = audioStream->safeRelease();
        // safeRelease will only fail if called illegally, for example, from a callback.
        result = audioStream->safeReleaseClose();
        // safeReleaseClose will only fail if called illegally, for example, from a callback.
        // That would result in deleting an active stream, which would cause a crash.
        if (result != AAUDIO_OK) {
            ALOGW("%s(s#%u) failed. Close it from another thread.",
                  __func__, id);
        } else {
            audioStream->unregisterPlayerBase();
             // Mark CLOSED to keep destructors from asserting.
            audioStream->closeFinal();
            delete audioStream;
        }
        ALOGD("%s(s#%u) returned %d ---------", __func__, id, result);
+13 −2
Original line number Diff line number Diff line
@@ -301,18 +301,29 @@ aaudio_result_t AudioStream::safeStop() {
}

aaudio_result_t AudioStream::safeRelease() {
    // This get temporarily unlocked in the release() when joining callback threads.
    // This get temporarily unlocked in the MMAP release() when joining callback threads.
    std::lock_guard<std::mutex> lock(mStreamLock);
    if (collidesWithCallback()) {
        ALOGE("%s cannot be called from a callback!", __func__);
        return AAUDIO_ERROR_INVALID_STATE;
    }
    if (getState() == AAUDIO_STREAM_STATE_CLOSING) {
    if (getState() == AAUDIO_STREAM_STATE_CLOSING) { // already released?
        return AAUDIO_OK;
    }
    return release_l();
}

aaudio_result_t AudioStream::safeReleaseClose() {
    // This get temporarily unlocked in the MMAP release() when joining callback threads.
    std::lock_guard<std::mutex> lock(mStreamLock);
    if (collidesWithCallback()) {
        ALOGE("%s cannot be called from a callback!", __func__);
        return AAUDIO_ERROR_INVALID_STATE;
    }
    releaseCloseFinal();
    return AAUDIO_OK;
}

void AudioStream::setState(aaudio_stream_state_t state) {
    ALOGD("%s(s#%d) from %d to %d", __func__, getId(), mState, state);
    // Track transition to DISCONNECTED state.
+36 −8
Original line number Diff line number Diff line
@@ -117,6 +117,17 @@ public:
    virtual void logOpen();
    void logReleaseBufferState();

    /* Note about naming for "release"  and "close" related methods.
     *
     * These names are intended to match the public AAudio API.
     * The original AAudio API had an AAudioStream_close() function that
     * released the hardware and deleted the stream. That made it difficult
     * because apps want to release the HW ASAP but are not in a rush to delete
     * the stream object. So in R we added an AAudioStream_release() function
     * that just released the hardware.
     * The AAudioStream_close() method releases if needed and then closes.
     */

    /**
     * Free any hardware or system resources from the open() call.
     * It is safe to call release_l() multiple times.
@@ -126,22 +137,27 @@ public:
        return AAUDIO_OK;
    }

    aaudio_result_t closeFinal() {
    /**
     * Free any resources not already freed by release_l().
     * Assume release_l() already called.
     */
    virtual void close_l() {
        // Releasing the stream will set the state to CLOSING.
        assert(getState() == AAUDIO_STREAM_STATE_CLOSING);
        // setState() prevents a transition from CLOSING to any state other than CLOSED.
        // State is checked by destructor.
        setState(AAUDIO_STREAM_STATE_CLOSED);
        return AAUDIO_OK;
    }

    /**
     * Release then close the stream.
     * @return AAUDIO_OK or negative error.
     */
    aaudio_result_t releaseCloseFinal() {
        aaudio_result_t result = release_l(); // TODO review locking
        if (result == AAUDIO_OK) {
          result = closeFinal();
    void releaseCloseFinal() {
        if (getState() != AAUDIO_STREAM_STATE_CLOSING) { // not already released?
            // Ignore result and keep closing.
            (void) release_l();
        }
        return result;
        close_l();
    }

    // This is only used to identify a stream in the logs without
@@ -395,8 +411,20 @@ public:
     */
    aaudio_result_t systemStopFromCallback();

    /**
     * Safely RELEASE a stream after taking mStreamLock and checking
     * to make sure we are not being called from a callback.
     * @return AAUDIO_OK or a negative error
     */
    aaudio_result_t safeRelease();

    /**
     * Safely RELEASE and CLOSE a stream after taking mStreamLock and checking
     * to make sure we are not being called from a callback.
     * @return AAUDIO_OK or a negative error
     */
    aaudio_result_t safeReleaseClose();

protected:

    // PlayerBase allows the system to control the stream volume.
+8 −2
Original line number Diff line number Diff line
@@ -293,14 +293,20 @@ aaudio_result_t AudioStreamRecord::release_l() {
    if (getState() != AAUDIO_STREAM_STATE_CLOSING) {
        mAudioRecord->removeAudioDeviceCallback(mDeviceCallback);
        logReleaseBufferState();
        mAudioRecord.clear();
        mFixedBlockWriter.close();
        // Data callbacks may still be running!
        return AudioStream::release_l();
    } else {
        return AAUDIO_OK; // already released
    }
}

void AudioStreamRecord::close_l() {
    // Stop callbacks before deleting mFixedBlockWriter memory.
    mAudioRecord.clear();
    mFixedBlockWriter.close();
    AudioStream::close_l();
}

const void * AudioStreamRecord::maybeConvertDeviceData(const void *audioData, int32_t numFrames) {
    if (mFormatConversionBufferFloat.get() != nullptr) {
        LOG_ALWAYS_FATAL_IF(numFrames > mFormatConversionBufferSizeInFrames,
Loading