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

Commit 638f45b7 authored by Andy Hung's avatar Andy Hung
Browse files

StreamHalHidl: Use atomic_wp and make pointers const 

Avoid concurrency issues with modification of wp.

Test: basic audio
Bug: 177278988
Change-Id: I5b968a1dad24e1d6c18260884d35c3170661da01
parent b3bd0a07

media/libaudiohal/Android.bp

+1 −0
Original line number Diff line number Diff line
@@ -31,6 +31,7 @@ cc_library_shared { 
    header_libs: [

        "libaudiohal_headers",

        "libbase_headers",

        "libmediautils_headers",

    ]

}

media/libaudiohal/impl/StreamHalHidl.cpp

+10 −18
Original line number Diff line number Diff line
@@ -61,10 +61,6 @@ StreamHalHidl::StreamHalHidl(IStream *stream) 
    }

}



StreamHalHidl::~StreamHalHidl() {

    mStream = nullptr;

}



status_t StreamHalHidl::getSampleRate(uint32_t *rate) {

    if (!mStream) return NO_INIT;

    return processReturn("getSampleRate", mStream->getSampleRate(), rate);
@@ -286,7 +282,7 @@ struct StreamOutCallback : public IStreamOutCallback { 
    }



  private:

    wp<StreamOutHalHidl> mStream;

    const wp<StreamOutHalHidl> mStream;

};



}  // namespace
@@ -297,21 +293,19 @@ StreamOutHalHidl::StreamOutHalHidl(const sp<IStreamOut>& stream) 


StreamOutHalHidl::~StreamOutHalHidl() {

    if (mStream != 0) {

        if (mCallback.unsafe_get()) {

        if (mCallback.load().unsafe_get()) {

            processReturn("clearCallback", mStream->clearCallback());

        }

#if MAJOR_VERSION >= 6

        if (mEventCallback.unsafe_get() != nullptr) {

        if (mEventCallback.load().unsafe_get() != nullptr) {

            processReturn("setEventCallback",

                    mStream->setEventCallback(nullptr));

        }

#endif

        processReturn("close", mStream->close());

        mStream.clear();

    }

    mCallback.clear();

    mEventCallback.clear();

    hardware::IPCThreadState::self()->flushCommands();

    mCallback = nullptr;

    mEventCallback = nullptr;

    if (mEfGroup) {

        EventFlag::deleteEventFlag(&mEfGroup);

    }
@@ -364,7 +358,7 @@ status_t StreamOutHalHidl::write(const void *buffer, size_t bytes, size_t *writt 


    if (bytes == 0 && !mDataMQ) {

        // Can't determine the size for the MQ buffer. Wait for a non-empty write request.

        ALOGW_IF(mCallback.unsafe_get(), "First call to async write with 0 bytes");

        ALOGW_IF(mCallback.load().unsafe_get(), "First call to async write with 0 bytes");

        return OK;

    }
@@ -680,28 +674,28 @@ status_t StreamOutHalHidl::setEventCallback( 
#endif



void StreamOutHalHidl::onWriteReady() {

    sp<StreamOutHalInterfaceCallback> callback = mCallback.promote();

    sp<StreamOutHalInterfaceCallback> callback = mCallback.load().promote();

    if (callback == 0) return;

    ALOGV("asyncCallback onWriteReady");

    callback->onWriteReady();

}



void StreamOutHalHidl::onDrainReady() {

    sp<StreamOutHalInterfaceCallback> callback = mCallback.promote();

    sp<StreamOutHalInterfaceCallback> callback = mCallback.load().promote();

    if (callback == 0) return;

    ALOGV("asyncCallback onDrainReady");

    callback->onDrainReady();

}



void StreamOutHalHidl::onError() {

    sp<StreamOutHalInterfaceCallback> callback = mCallback.promote();

    sp<StreamOutHalInterfaceCallback> callback = mCallback.load().promote();

    if (callback == 0) return;

    ALOGV("asyncCallback onError");

    callback->onError();

}



void StreamOutHalHidl::onCodecFormatChanged(const std::basic_string<uint8_t>& metadataBs) {

    sp<StreamOutHalInterfaceEventCallback> callback = mEventCallback.promote();

    sp<StreamOutHalInterfaceEventCallback> callback = mEventCallback.load().promote();

    if (callback == nullptr) return;

    ALOGV("asyncCodecFormatCallback %s", __func__);

    callback->onCodecFormatChanged(metadataBs);
@@ -715,8 +709,6 @@ StreamInHalHidl::StreamInHalHidl(const sp<IStreamIn>& stream) 
StreamInHalHidl::~StreamInHalHidl() {

    if (mStream != 0) {

        processReturn("close", mStream->close());

        mStream.clear();

        hardware::IPCThreadState::self()->flushCommands();

    }

    if (mEfGroup) {

        EventFlag::deleteEventFlag(&mEfGroup);

media/libaudiohal/impl/StreamHalHidl.h

+20 −8
Original line number Diff line number Diff line
@@ -25,6 +25,7 @@ 
#include <fmq/EventFlag.h>

#include <fmq/MessageQueue.h>

#include <media/audiohal/StreamHalInterface.h>

#include <mediautils/Synchronization.h>



#include "ConversionHelperHidl.h"

#include "StreamPowerLog.h"
@@ -100,9 +101,6 @@ class StreamHalHidl : public virtual StreamHalInterface, public ConversionHelper 
    // Subclasses can not be constructed directly by clients.

    explicit StreamHalHidl(IStream *stream);



    // The destructor automatically closes the stream.

    virtual ~StreamHalHidl();



    status_t getCachedBufferSize(size_t *size);



    bool requestHalThreadPriority(pid_t threadPid, pid_t threadId);
@@ -112,7 +110,7 @@ class StreamHalHidl : public virtual StreamHalInterface, public ConversionHelper 


  private:

    const int HAL_THREAD_PRIORITY_DEFAULT = -1;

    IStream *mStream;

    IStream * const mStream;

    int mHalThreadPriority;

    size_t mCachedBufferSize;

};
@@ -184,9 +182,16 @@ class StreamOutHalHidl : public StreamOutHalInterface, public StreamHalHidl { 
    typedef MessageQueue<uint8_t, hardware::kSynchronizedReadWrite> DataMQ;

    typedef MessageQueue<WriteStatus, hardware::kSynchronizedReadWrite> StatusMQ;



    wp<StreamOutHalInterfaceCallback> mCallback;

    wp<StreamOutHalInterfaceEventCallback> mEventCallback;

    sp<IStreamOut> mStream;

    // Do not move the Defer.  This should be the first member variable in the class;

    // thus the last member destructor called upon instance destruction.

    //

    // The last step is to flush all binder commands so that the AudioFlinger

    // may recognize the deletion of IStreamOut (mStream) with less delay. See b/35394629.

    mediautils::Defer mLast{[]() { hardware::IPCThreadState::self()->flushCommands(); }};



    mediautils::atomic_wp<StreamOutHalInterfaceCallback> mCallback;

    mediautils::atomic_wp<StreamOutHalInterfaceEventCallback> mEventCallback;

    const sp<IStreamOut> mStream;

    std::unique_ptr<CommandMQ> mCommandMQ;

    std::unique_ptr<DataMQ> mDataMQ;

    std::unique_ptr<StatusMQ> mStatusMQ;
@@ -242,7 +247,14 @@ class StreamInHalHidl : public StreamInHalInterface, public StreamHalHidl { 
    typedef MessageQueue<uint8_t, hardware::kSynchronizedReadWrite> DataMQ;

    typedef MessageQueue<ReadStatus, hardware::kSynchronizedReadWrite> StatusMQ;



    sp<IStreamIn> mStream;

    // Do not move the Defer.  This should be the first member variable in the class;

    // thus the last member destructor called upon instance destruction.

    //

    // The last step is to flush all binder commands so that the AudioFlinger

    // may recognize the deletion of IStreamIn (mStream) with less delay. See b/35394629.

    mediautils::Defer mLast{[]() { hardware::IPCThreadState::self()->flushCommands(); }};



    const sp<IStreamIn> mStream;

    std::unique_ptr<CommandMQ> mCommandMQ;

    std::unique_ptr<DataMQ> mDataMQ;

    std::unique_ptr<StatusMQ> mStatusMQ;

media/utils/include/mediautils/Synchronization.h

+14 −0
Original line number Diff line number Diff line
@@ -115,5 +115,19 @@ template <typename T> 
using atomic_sp = LockItem<

        ::android::sp<T>, std::mutex, LockItem<::android::sp<T>>::FLAG_DTOR_OUT_OF_LOCK>;



/**

 * Defers a function to run in the RAII destructor.

 * A C++ implementation of Go _defer_ https://golangr.com/defer/.

 */

class Defer {

public:

    template <typename U>

    explicit Defer(U &&f) : mThunk(std::forward<U>(f)) {}

    ~Defer() { mThunk(); }



private:

    const std::function<void()> mThunk;

};



} // namespace android::mediautils