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

Commit 98d6d922 authored by Phil Burk's avatar Phil Burk
Browse files

aaudio: stop calling virtual methods from destructor 

Was calling close(), which is abstract and virtual.
This may have been related to some audioserver crashes.
Also cleaned up some strong pointer handling.

Bug: 63390734
Bug: 63353455
Test: run CTS nativemedia/aaudio many times
Change-Id: Ib95aed60a64771b64395c67f0921c67146f9d10f
parent 014a56b5

services/oboeservice/AAudioService.cpp

+4 −3
Original line number Diff line number Diff line
@@ -150,17 +150,18 @@ aaudio_handle_t AAudioService::openStream(const aaudio::AAudioStreamRequest &req 
}



aaudio_result_t AAudioService::closeStream(aaudio_handle_t streamHandle) {

    // Check permission first.

    AAudioServiceStreamBase *serviceStream = convertHandleToServiceStream(streamHandle);

    // Check permission and ownership first.

    sp<AAudioServiceStreamBase> serviceStream = convertHandleToServiceStream(streamHandle);

    if (serviceStream == nullptr) {

        ALOGE("AAudioService::startStream(), illegal stream handle = 0x%0x", streamHandle);

        return AAUDIO_ERROR_INVALID_HANDLE;

    }



    ALOGD("AAudioService.closeStream(0x%08X)", streamHandle);

    // Remove handle from tracker so that we cannot look up the raw address any more.

    serviceStream = (AAudioServiceStreamBase *)

            mHandleTracker.remove(AAUDIO_HANDLE_TYPE_STREAM,

                                  streamHandle);

    ALOGD("AAudioService.closeStream(0x%08X)", streamHandle);

    if (serviceStream != nullptr) {

        serviceStream->close();

        pid_t pid = serviceStream->getOwnerProcessId();

services/oboeservice/AAudioServiceStreamBase.cpp

+24 −10
Original line number Diff line number Diff line
@@ -41,8 +41,11 @@ AAudioServiceStreamBase::AAudioServiceStreamBase() 
}



AAudioServiceStreamBase::~AAudioServiceStreamBase() {

    close();

    ALOGD("AAudioServiceStreamBase::~AAudioServiceStreamBase() destroyed %p", this);

    ALOGD("AAudioServiceStreamBase::~AAudioServiceStreamBase() destroying %p", this);

    // If the stream is deleted without closing then audio resources will leak.

    // Not being closed here would indicate an internal error. So we want to find this ASAP.

    LOG_ALWAYS_FATAL_IF(mState != AAUDIO_STREAM_STATE_CLOSED,

                        "service stream not closed, state = %d", mState);

}



std::string AAudioServiceStreamBase::dump() const {
@@ -71,11 +74,13 @@ aaudio_result_t AAudioServiceStreamBase::open(const aaudio::AAudioStreamRequest 
}



aaudio_result_t AAudioServiceStreamBase::close() {

    stop();

    if (mState != AAUDIO_STREAM_STATE_CLOSED) {

        stopTimestampThread();

        std::lock_guard<std::mutex> lock(mLockUpMessageQueue);

        delete mUpMessageQueue;

        mUpMessageQueue = nullptr;



        mState = AAUDIO_STREAM_STATE_CLOSED;

    }

    return AAUDIO_OK;

}
@@ -106,9 +111,8 @@ aaudio_result_t AAudioServiceStreamBase::stop() { 
    aaudio_result_t result = AAUDIO_OK;

    if (isRunning()) {

        // TODO wait for data to be played out

        sendCurrentTimestamp();

        mThreadEnabled.store(false);

        result = mAAudioThread.stop();

        sendCurrentTimestamp(); // warning - this calls a virtual function

        result = stopTimestampThread();

        if (result != AAUDIO_OK) {

            disconnect();

            return result;
@@ -119,6 +123,15 @@ aaudio_result_t AAudioServiceStreamBase::stop() { 
    return result;

}



aaudio_result_t AAudioServiceStreamBase::stopTimestampThread() {

    aaudio_result_t result = AAUDIO_OK;

    // clear flag that tells thread to loop

    if (mThreadEnabled.exchange(false)) {

        result = mAAudioThread.stop();

    }

    return result;

}



aaudio_result_t AAudioServiceStreamBase::flush() {

    sendServiceEvent(AAUDIO_SERVICE_EVENT_FLUSHED);

    mState = AAUDIO_STREAM_STATE_FLUSHED;
@@ -141,6 +154,7 @@ void AAudioServiceStreamBase::run() { 
            nextTime = timestampScheduler.nextAbsoluteTime();

        } else  {

            // Sleep until it is time to send the next timestamp.

            // TODO Wait for a signal with a timeout so that we can stop more quickly.

            AudioClock::sleepUntilNanoTime(nextTime);

        }

    }

services/oboeservice/AAudioServiceStreamBase.h

+3 −0
Original line number Diff line number Diff line
@@ -78,6 +78,8 @@ public: 
     */

    virtual aaudio_result_t stop();



    aaudio_result_t stopTimestampThread();



    /**

     *  Discard any data held by the underlying HAL or Service.

     */
@@ -143,6 +145,7 @@ public: 
    }



protected:



    aaudio_result_t writeUpMessageQueue(AAudioServiceMessage *command);



    aaudio_result_t sendCurrentTimestamp();

services/oboeservice/AAudioServiceStreamMMAP.cpp

+11 −9
Original line number Diff line number Diff line
@@ -49,16 +49,18 @@ AAudioServiceStreamMMAP::AAudioServiceStreamMMAP(uid_t serviceUid) 
        , mCachedUserId(serviceUid) {

}



AAudioServiceStreamMMAP::~AAudioServiceStreamMMAP() {

    close();

aaudio_result_t AAudioServiceStreamMMAP::close() {

    if (mState == AAUDIO_STREAM_STATE_CLOSED) {

        return AAUDIO_OK;

    }



aaudio_result_t AAudioServiceStreamMMAP::close() {

    if (mMmapStream != 0) {

        mMmapStream.clear(); // TODO review. Is that all we have to do?

        // Apparently the above close is asynchronous. An attempt to open a new device

        // right after a close can fail. Also some callbacks may still be in flight!

        // FIXME Make closing synchronous.

        AudioClock::sleepForNanos(100 * AAUDIO_NANOS_PER_MILLISECOND);

    }



    if (mAudioDataFileDescriptor != -1) {

        ::close(mAudioDataFileDescriptor);

services/oboeservice/AAudioServiceStreamMMAP.h

+1 −2
Original line number Diff line number Diff line
@@ -44,8 +44,7 @@ class AAudioServiceStreamMMAP 


public:

    AAudioServiceStreamMMAP(uid_t serviceUid);

    virtual ~AAudioServiceStreamMMAP();



    virtual ~AAudioServiceStreamMMAP() = default;



    aaudio_result_t open(const aaudio::AAudioStreamRequest &request,

                                 aaudio::AAudioStreamConfiguration &configurationOutput) override;