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

Commit 10710e46 authored by Phil Burk's avatar Phil Burk
Browse files

aaudio: lock transport methods

The start/pause/stop/flush/close and other binder methods
need to be thread safe. They do not need to run
in parallel. So a lock was added for each.

Where virtual methods are needed, the locked method calls
a corresponding _l submethod, eg. stop() calls stop_l().

The close logic was also simplified because the "pending"
technique is not needed now that we have the locks.
It was only needed because a close could have occured
while in the middle of another method.

Bug: 153358911
Test: adb logcat *:F
Test: in another window:  test_binder_start_storm
Test: There should be no fatal error in the logcat.
Test: atest CtsNativeMediaAAudioTestCases
Change-Id: I5920cf78af4501856756c5c2fc8e77758232508a
Merged-In: I5920cf78af4501856756c5c2fc8e77758232508a
parent f8c7ecf6
Loading
Loading
Loading
Loading
+28 −81
Original line number Diff line number Diff line
@@ -23,7 +23,6 @@
#include <sstream>

#include <aaudio/AAudio.h>
#include <mediautils/SchedulingPolicyService.h>
#include <mediautils/ServiceUtilities.h>
#include <utils/String16.h>

@@ -144,28 +143,6 @@ aaudio_handle_t AAudioService::openStream(const aaudio::AAudioStreamRequest &req
    }
}

// If a close request is pending then close the stream
bool AAudioService::releaseStream(const sp<AAudioServiceStreamBase> &serviceStream) {
    bool closed = false;
    // decrementAndRemoveStreamByHandle() uses a lock so that if there are two simultaneous closes
    // then only one will get the pointer and do the close.
    sp<AAudioServiceStreamBase> foundStream = mStreamTracker.decrementAndRemoveStreamByHandle(
            serviceStream->getHandle());
    if (foundStream.get() != nullptr) {
        foundStream->close();
        pid_t pid = foundStream->getOwnerProcessId();
        AAudioClientTracker::getInstance().unregisterClientStream(pid, foundStream);
        closed = true;
    }
    return closed;
}

aaudio_result_t AAudioService::checkForPendingClose(
        const sp<AAudioServiceStreamBase> &serviceStream,
        aaudio_result_t defaultResult) {
    return releaseStream(serviceStream) ? AAUDIO_ERROR_INVALID_STATE : defaultResult;
}

aaudio_result_t AAudioService::closeStream(aaudio_handle_t streamHandle) {
    // Check permission and ownership first.
    sp<AAudioServiceStreamBase> serviceStream = convertHandleToServiceStream(streamHandle);
@@ -174,17 +151,20 @@ aaudio_result_t AAudioService::closeStream(aaudio_handle_t streamHandle) {
        return AAUDIO_ERROR_INVALID_HANDLE;
    }

    // This is protected by a lock in AAudioClientTracker.
    // It is safe to unregister the same stream twice.
    pid_t pid = serviceStream->getOwnerProcessId();
    AAudioClientTracker::getInstance().unregisterClientStream(pid, serviceStream);
    // This is protected by a lock in mStreamTracker.
    // It is safe to remove the same stream twice.
    mStreamTracker.removeStreamByHandle(streamHandle);

    serviceStream->markCloseNeeded();
    (void) releaseStream(serviceStream);
    return AAUDIO_OK;
    return serviceStream->close();
}

sp<AAudioServiceStreamBase> AAudioService::convertHandleToServiceStream(
        aaudio_handle_t streamHandle) {
    sp<AAudioServiceStreamBase> serviceStream = mStreamTracker.getStreamByHandleAndIncrement(
    sp<AAudioServiceStreamBase> serviceStream = mStreamTracker.getStreamByHandle(
            streamHandle);
    if (serviceStream.get() != nullptr) {
        // Only allow owner or the aaudio service to access the stream.
@@ -197,8 +177,6 @@ sp<AAudioServiceStreamBase> AAudioService::convertHandleToServiceStream(
        if (!allowed) {
            ALOGE("AAudioService: calling uid %d cannot access stream 0x%08X owned by %d",
                  callingUserId, streamHandle, ownerUserId);
            // We incremented the reference count so we must check if it needs to be closed.
            checkForPendingClose(serviceStream, AAUDIO_OK);
            serviceStream.clear();
        }
    }
@@ -213,94 +191,64 @@ aaudio_result_t AAudioService::getStreamDescription(
        ALOGE("getStreamDescription(), illegal stream handle = 0x%0x", streamHandle);
        return AAUDIO_ERROR_INVALID_HANDLE;
    }

    aaudio_result_t result = serviceStream->getDescription(parcelable);
    // parcelable.dump();
    return checkForPendingClose(serviceStream, result);
    return serviceStream->getDescription(parcelable);
}

aaudio_result_t AAudioService::startStream(aaudio_handle_t streamHandle) {
    sp<AAudioServiceStreamBase> serviceStream = convertHandleToServiceStream(streamHandle);
    if (serviceStream.get() == nullptr) {
        ALOGE("startStream(), illegal stream handle = 0x%0x", streamHandle);
        ALOGW("%s(), invalid streamHandle = 0x%0x", __func__, streamHandle);
        return AAUDIO_ERROR_INVALID_HANDLE;
    }

    aaudio_result_t result = serviceStream->start();
    return checkForPendingClose(serviceStream, result);
    return serviceStream->start();
}

aaudio_result_t AAudioService::pauseStream(aaudio_handle_t streamHandle) {
    sp<AAudioServiceStreamBase> serviceStream = convertHandleToServiceStream(streamHandle);
    if (serviceStream.get() == nullptr) {
        ALOGE("pauseStream(), illegal stream handle = 0x%0x", streamHandle);
        ALOGW("%s(), invalid streamHandle = 0x%0x", __func__, streamHandle);
        return AAUDIO_ERROR_INVALID_HANDLE;
    }
    aaudio_result_t result = serviceStream->pause();
    return checkForPendingClose(serviceStream, result);
    return serviceStream->pause();
}

aaudio_result_t AAudioService::stopStream(aaudio_handle_t streamHandle) {
    sp<AAudioServiceStreamBase> serviceStream = convertHandleToServiceStream(streamHandle);
    if (serviceStream.get() == nullptr) {
        ALOGE("stopStream(), illegal stream handle = 0x%0x", streamHandle);
        ALOGW("%s(), invalid streamHandle = 0x%0x", __func__, streamHandle);
        return AAUDIO_ERROR_INVALID_HANDLE;
    }
    aaudio_result_t result = serviceStream->stop();
    return checkForPendingClose(serviceStream, result);
    return serviceStream->stop();
}

aaudio_result_t AAudioService::flushStream(aaudio_handle_t streamHandle) {
    sp<AAudioServiceStreamBase> serviceStream = convertHandleToServiceStream(streamHandle);
    if (serviceStream.get() == nullptr) {
        ALOGE("flushStream(), illegal stream handle = 0x%0x", streamHandle);
        ALOGW("%s(), invalid streamHandle = 0x%0x", __func__, streamHandle);
        return AAUDIO_ERROR_INVALID_HANDLE;
    }
    aaudio_result_t result = serviceStream->flush();
    return checkForPendingClose(serviceStream, result);
    return serviceStream->flush();
}

aaudio_result_t AAudioService::registerAudioThread(aaudio_handle_t streamHandle,
                                                   pid_t clientThreadId,
                                                   int64_t periodNanoseconds) {
    aaudio_result_t result = AAUDIO_OK;
                                                   int64_t /* periodNanoseconds */) {
    sp<AAudioServiceStreamBase> serviceStream = convertHandleToServiceStream(streamHandle);
    if (serviceStream.get() == nullptr) {
        ALOGE("registerAudioThread(), illegal stream handle = 0x%0x", streamHandle);
        ALOGW("%s(), invalid streamHandle = 0x%0x", __func__, streamHandle);
        return AAUDIO_ERROR_INVALID_HANDLE;
    }
    if (serviceStream->getRegisteredThread() != AAudioServiceStreamBase::ILLEGAL_THREAD_ID) {
        ALOGE("AAudioService::registerAudioThread(), thread already registered");
        result = AAUDIO_ERROR_INVALID_STATE;
    } else {
        const pid_t ownerPid = IPCThreadState::self()->getCallingPid(); // TODO review
        serviceStream->setRegisteredThread(clientThreadId);
        int err = android::requestPriority(ownerPid, clientThreadId,
                                           DEFAULT_AUDIO_PRIORITY, true /* isForApp */);
        if (err != 0) {
            ALOGE("AAudioService::registerAudioThread(%d) failed, errno = %d, priority = %d",
                  clientThreadId, errno, DEFAULT_AUDIO_PRIORITY);
            result = AAUDIO_ERROR_INTERNAL;
        }
    }
    return checkForPendingClose(serviceStream, result);
    return serviceStream->registerAudioThread(clientThreadId, DEFAULT_AUDIO_PRIORITY);
}

aaudio_result_t AAudioService::unregisterAudioThread(aaudio_handle_t streamHandle,
                                                     pid_t clientThreadId) {
    aaudio_result_t result = AAUDIO_OK;
    sp<AAudioServiceStreamBase> serviceStream = convertHandleToServiceStream(streamHandle);
    if (serviceStream.get() == nullptr) {
        ALOGE("%s(), illegal stream handle = 0x%0x", __func__, streamHandle);
        ALOGW("%s(), invalid streamHandle = 0x%0x", __func__, streamHandle);
        return AAUDIO_ERROR_INVALID_HANDLE;
    }
    if (serviceStream->getRegisteredThread() != clientThreadId) {
        ALOGE("%s(), wrong thread", __func__);
        result = AAUDIO_ERROR_ILLEGAL_ARGUMENT;
    } else {
        serviceStream->setRegisteredThread(0);
    }
    return checkForPendingClose(serviceStream, result);
    return serviceStream->unregisterAudioThread(clientThreadId);
}

aaudio_result_t AAudioService::startClient(aaudio_handle_t streamHandle,
@@ -308,22 +256,20 @@ aaudio_result_t AAudioService::startClient(aaudio_handle_t streamHandle,
                                  audio_port_handle_t *clientHandle) {
    sp<AAudioServiceStreamBase> serviceStream = convertHandleToServiceStream(streamHandle);
    if (serviceStream.get() == nullptr) {
        ALOGE("%s(), illegal stream handle = 0x%0x", __func__, streamHandle);
        ALOGW("%s(), invalid streamHandle = 0x%0x", __func__, streamHandle);
        return AAUDIO_ERROR_INVALID_HANDLE;
    }
    aaudio_result_t result = serviceStream->startClient(client, clientHandle);
    return checkForPendingClose(serviceStream, result);
    return serviceStream->startClient(client, clientHandle);
}

aaudio_result_t AAudioService::stopClient(aaudio_handle_t streamHandle,
                                          audio_port_handle_t portHandle) {
    sp<AAudioServiceStreamBase> serviceStream = convertHandleToServiceStream(streamHandle);
    if (serviceStream.get() == nullptr) {
        ALOGE("%s(), illegal stream handle = 0x%0x", __func__, streamHandle);
        ALOGW("%s(), invalid streamHandle = 0x%0x", __func__, streamHandle);
        return AAUDIO_ERROR_INVALID_HANDLE;
    }
    aaudio_result_t result = serviceStream->stopClient(portHandle);
    return checkForPendingClose(serviceStream, result);
    return serviceStream->stopClient(portHandle);
}

// This is only called internally when AudioFlinger wants to tear down a stream.
@@ -331,12 +277,13 @@ aaudio_result_t AAudioService::stopClient(aaudio_handle_t streamHandle,
aaudio_result_t AAudioService::disconnectStreamByPortHandle(audio_port_handle_t portHandle) {
    ALOGD("%s(%d) called", __func__, portHandle);
    sp<AAudioServiceStreamBase> serviceStream =
            mStreamTracker.findStreamByPortHandleAndIncrement(portHandle);
            mStreamTracker.findStreamByPortHandle(portHandle);
    if (serviceStream.get() == nullptr) {
        ALOGE("%s(), could not find stream with portHandle = %d", __func__, portHandle);
        return AAUDIO_ERROR_INVALID_HANDLE;
    }
    // This is protected by a lock and will just return if already stopped.
    aaudio_result_t result = serviceStream->stop();
    serviceStream->disconnect();
    return checkForPendingClose(serviceStream, result);
    return result;
}
+0 −7
Original line number Diff line number Diff line
@@ -95,13 +95,6 @@ private:
    sp<aaudio::AAudioServiceStreamBase> convertHandleToServiceStream(
            aaudio::aaudio_handle_t streamHandle);



    bool releaseStream(const sp<aaudio::AAudioServiceStreamBase> &serviceStream);

    aaudio_result_t checkForPendingClose(const sp<aaudio::AAudioServiceStreamBase> &serviceStream,
                                         aaudio_result_t defaultResult);

    android::AudioClient            mAudioClient;

    aaudio::AAudioStreamTracker     mStreamTracker;
+8 −3
Original line number Diff line number Diff line
@@ -87,15 +87,20 @@ bool AAudioServiceEndpoint::isStreamRegistered(audio_port_handle_t portHandle) {
}

void AAudioServiceEndpoint::disconnectRegisteredStreams() {
    std::vector<android::sp<AAudioServiceStreamBase>> streamsToDisconnect;
    {
        std::lock_guard<std::mutex> lock(mLockStreams);
        mRegisteredStreams.swap(streamsToDisconnect);
    }
    // Stop and disconnect outside mLockStreams to avoid reverse
    // ordering of AAudioServiceStreamBase::mLock and mLockStreams
    mConnected.store(false);
    for (const auto& stream : mRegisteredStreams) {
    for (const auto& stream : streamsToDisconnect) {
        ALOGD("disconnectRegisteredStreams() stop and disconnect port %d",
              stream->getPortHandle());
        stream->stop();
        stream->disconnect();
    }
    mRegisteredStreams.clear();
}

aaudio_result_t AAudioServiceEndpoint::registerStream(sp<AAudioServiceStreamBase>stream) {
+1 −3
Original line number Diff line number Diff line
@@ -166,13 +166,11 @@ aaudio_result_t AAudioServiceEndpointShared::startStream(sp<AAudioServiceStreamB

aaudio_result_t AAudioServiceEndpointShared::stopStream(sp<AAudioServiceStreamBase> sharedStream,
                                                        audio_port_handle_t clientHandle) {
    // Don't lock here because the disconnectRegisteredStreams also uses the lock.

    // Ignore result.
    (void) getStreamInternal()->stopClient(clientHandle);

    if (--mRunningStreamCount == 0) { // atomic
        stopSharingThread();
        stopSharingThread(); // the sharing thread locks mLockStreams
        getStreamInternal()->requestStop();
    }
    return AAUDIO_OK;
+84 −10
Original line number Diff line number Diff line
@@ -22,6 +22,8 @@
#include <iostream>
#include <mutex>

#include <mediautils/SchedulingPolicyService.h>

#include "binding/IAAudioService.h"
#include "binding/AAudioServiceMessage.h"
#include "utility/AudioClock.h"
@@ -126,12 +128,17 @@ error:
}

aaudio_result_t AAudioServiceStreamBase::close() {
    std::lock_guard<std::mutex> lock(mLock);
    return close_l();
}

aaudio_result_t AAudioServiceStreamBase::close_l() {
    aaudio_result_t result = AAUDIO_OK;
    if (getState() == AAUDIO_STREAM_STATE_CLOSED) {
        return AAUDIO_OK;
    }

    stop();
    stop_l();

    sp<AAudioServiceEndpoint> endpoint = mServiceEndpointWeak.promote();
    if (endpoint == nullptr) {
@@ -142,7 +149,7 @@ aaudio_result_t AAudioServiceStreamBase::close() {
        endpointManager.closeEndpoint(endpoint);

        // AAudioService::closeStream() prevents two threads from closing at the same time.
        mServiceEndpoint.clear(); // endpoint will hold the pointer until this method returns.
        mServiceEndpoint.clear(); // endpoint will hold the pointer after this method returns.
    }

    {
@@ -172,7 +179,14 @@ aaudio_result_t AAudioServiceStreamBase::startDevice() {
 * An AAUDIO_SERVICE_EVENT_STARTED will be sent to the client when complete.
 */
aaudio_result_t AAudioServiceStreamBase::start() {
    aaudio_result_t result = AAUDIO_OK;
    std::lock_guard<std::mutex> lock(mLock);

    if (auto state = getState();
        state == AAUDIO_STREAM_STATE_CLOSED || state == AAUDIO_STREAM_STATE_DISCONNECTED) {
        ALOGW("%s() already CLOSED, returns INVALID_STATE, handle = %d",
                __func__, getHandle());
        return AAUDIO_ERROR_INVALID_STATE;
    }

    if (isRunning()) {
        return AAUDIO_OK;
@@ -185,7 +199,7 @@ aaudio_result_t AAudioServiceStreamBase::start() {
    mAtomicStreamTimestamp.clear();

    mClientHandle = AUDIO_PORT_HANDLE_NONE;
    result = startDevice();
    aaudio_result_t result = startDevice();
    if (result != AAUDIO_OK) goto error;

    // This should happen at the end of the start.
@@ -198,11 +212,16 @@ aaudio_result_t AAudioServiceStreamBase::start() {
    return result;

error:
    disconnect();
    disconnect_l();
    return result;
}

aaudio_result_t AAudioServiceStreamBase::pause() {
    std::lock_guard<std::mutex> lock(mLock);
    return pause_l();
}

aaudio_result_t AAudioServiceStreamBase::pause_l() {
    aaudio_result_t result = AAUDIO_OK;
    if (!isRunning()) {
        return result;
@@ -214,7 +233,7 @@ aaudio_result_t AAudioServiceStreamBase::pause() {

    result = stopTimestampThread();
    if (result != AAUDIO_OK) {
        disconnect();
        disconnect_l();
        return result;
    }

@@ -226,7 +245,7 @@ aaudio_result_t AAudioServiceStreamBase::pause() {
    result = endpoint->stopStream(this, mClientHandle);
    if (result != AAUDIO_OK) {
        ALOGE("%s() mServiceEndpoint returned %d, %s", __func__, result, getTypeText());
        disconnect(); // TODO should we return or pause Base first?
        disconnect_l(); // TODO should we return or pause Base first?
    }

    sendServiceEvent(AAUDIO_SERVICE_EVENT_PAUSED);
@@ -235,6 +254,11 @@ aaudio_result_t AAudioServiceStreamBase::pause() {
}

aaudio_result_t AAudioServiceStreamBase::stop() {
    std::lock_guard<std::mutex> lock(mLock);
    return stop_l();
}

aaudio_result_t AAudioServiceStreamBase::stop_l() {
    aaudio_result_t result = AAUDIO_OK;
    if (!isRunning()) {
        return result;
@@ -247,7 +271,7 @@ aaudio_result_t AAudioServiceStreamBase::stop() {
    sendCurrentTimestamp(); // warning - this calls a virtual function
    result = stopTimestampThread();
    if (result != AAUDIO_OK) {
        disconnect();
        disconnect_l();
        return result;
    }

@@ -260,7 +284,7 @@ aaudio_result_t AAudioServiceStreamBase::stop() {
    result = endpoint->stopStream(this, mClientHandle);
    if (result != AAUDIO_OK) {
        ALOGE("%s() stopStream returned %d, %s", __func__, result, getTypeText());
        disconnect();
        disconnect_l();
        // TODO what to do with result here?
    }

@@ -279,6 +303,7 @@ aaudio_result_t AAudioServiceStreamBase::stopTimestampThread() {
}

aaudio_result_t AAudioServiceStreamBase::flush() {
    std::lock_guard<std::mutex> lock(mLock);
    aaudio_result_t result = AAudio_isFlushAllowed(getState());
    if (result != AAUDIO_OK) {
        return result;
@@ -319,12 +344,60 @@ void AAudioServiceStreamBase::run() {
}

void AAudioServiceStreamBase::disconnect() {
    if (getState() != AAUDIO_STREAM_STATE_DISCONNECTED) {
    std::lock_guard<std::mutex> lock(mLock);
    disconnect_l();
}

void AAudioServiceStreamBase::disconnect_l() {
    if (getState() != AAUDIO_STREAM_STATE_DISCONNECTED
        && getState() != AAUDIO_STREAM_STATE_CLOSED) {
        sendServiceEvent(AAUDIO_SERVICE_EVENT_DISCONNECTED);
        setState(AAUDIO_STREAM_STATE_DISCONNECTED);
    }
}

aaudio_result_t AAudioServiceStreamBase::registerAudioThread(pid_t clientThreadId,
        int priority) {
    std::lock_guard<std::mutex> lock(mLock);
    aaudio_result_t result = AAUDIO_OK;
    if (getRegisteredThread() != AAudioServiceStreamBase::ILLEGAL_THREAD_ID) {
        ALOGE("AAudioService::registerAudioThread(), thread already registered");
        result = AAUDIO_ERROR_INVALID_STATE;
    } else {
        const pid_t ownerPid = IPCThreadState::self()->getCallingPid(); // TODO review
        setRegisteredThread(clientThreadId);
        int err = android::requestPriority(ownerPid, clientThreadId,
                                           priority, true /* isForApp */);
        if (err != 0) {
            ALOGE("AAudioService::registerAudioThread(%d) failed, errno = %d, priority = %d",
                  clientThreadId, errno, priority);
            result = AAUDIO_ERROR_INTERNAL;
        }
    }
    return result;
}

aaudio_result_t AAudioServiceStreamBase::unregisterAudioThread(pid_t clientThreadId) {
    std::lock_guard<std::mutex> lock(mLock);
    aaudio_result_t result = AAUDIO_OK;
    if (getRegisteredThread() != clientThreadId) {
        ALOGE("%s(), wrong thread", __func__);
        result = AAUDIO_ERROR_ILLEGAL_ARGUMENT;
    } else {
        setRegisteredThread(0);
    }
    return result;
}

void AAudioServiceStreamBase::setState(aaudio_stream_state_t state) {
    // CLOSED is a final state.
    if (mState != AAUDIO_STREAM_STATE_CLOSED) {
        mState = state;
    } else {
        ALOGW_IF(mState != state, "%s(%d) when already CLOSED", __func__, state);
    }
}

aaudio_result_t AAudioServiceStreamBase::sendServiceEvent(aaudio_service_event_t event,
                                                          double  dataDouble) {
    AAudioServiceMessage command;
@@ -422,6 +495,7 @@ aaudio_result_t AAudioServiceStreamBase::sendCurrentTimestamp() {
 * used to communicate with the underlying HAL or Service.
 */
aaudio_result_t AAudioServiceStreamBase::getDescription(AudioEndpointParcelable &parcelable) {
    std::lock_guard<std::mutex> lock(mLock);
    {
        std::lock_guard<std::mutex> lock(mUpMessageQueueLock);
        if (mUpMessageQueue == nullptr) {
Loading