Skip to content

Commit

Permalink
oboe: lock waitForStateChange for AAudio
Browse files Browse the repository at this point in the history
Call AAudio with timeout zero to avoid blocking.
Prevent crashes if close() called from another thread.

Possible fix for #406
  • Loading branch information
Phil Burk committed Mar 28, 2019
1 parent d8854b7 commit be13441
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 16 deletions.
77 changes: 64 additions & 13 deletions src/aaudio/AudioStreamAAudio.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@

#include "aaudio/AAudioLoader.h"
#include "aaudio/AudioStreamAAudio.h"
#include "common/AudioClock.h"
#include "common/OboeDebug.h"
#include "oboe/Utilities.h"

Expand Down Expand Up @@ -370,25 +371,75 @@ ResultWithValue<int32_t> AudioStreamAAudio::read(void *buffer,
}
}


// AAudioStream_waitForStateChange() can crash if it is waiting on a stream and that stream
// is closed from another thread. We do not want to lock the stream for the duration of the call.
// So we call AAudioStream_waitForStateChange() with a timeout of zero so that it will not block.
// Then we can do our own sleep with the lock unlocked.
Result AudioStreamAAudio::waitForStateChange(StreamState currentState,
StreamState *nextState,
int64_t timeoutNanoseconds) {
AAudioStream *stream = mAAudioStream.load();
if (stream != nullptr) {
Result oboeResult = (timeoutNanoseconds <= 0) ? Result::OK : Result::ErrorTimeout;
int64_t durationNanos = 20 * kNanosPerMillisecond; // arbitrary
aaudio_stream_state_t currentAAudioState = static_cast<aaudio_stream_state_t>(currentState);

aaudio_stream_state_t aaudioNextState;
aaudio_result_t result = AAUDIO_OK;
int64_t timeLeftNanos = timeoutNanoseconds;

mLock.lock();
while (true) {
// Do we still have an AAudio stream? If not then stream must have been closed.
AAudioStream *stream = mAAudioStream.load();
if (stream == nullptr) {
if (nextState != nullptr) {
*nextState = StreamState::Closed;
}
oboeResult = Result::ErrorClosed;
break;
}

aaudio_stream_state_t aaudioNextState;
aaudio_result_t result = mLibLoader->stream_waitForStateChange(
mAAudioStream,
static_cast<aaudio_stream_state_t>(currentState),
&aaudioNextState,
timeoutNanoseconds);
*nextState = static_cast<StreamState>(aaudioNextState);
return static_cast<Result>(result);
} else {
*nextState = StreamState::Closed;
// Update and query state change with no blocking.
result = mLibLoader->stream_waitForStateChange(
mAAudioStream,
currentAAudioState,
&aaudioNextState,
0); // timeout=0 for non-blocking
if (nextState != nullptr) {
*nextState = static_cast<StreamState>(aaudioNextState);
}
if (result != AAUDIO_OK) {
if (result == AAUDIO_ERROR_TIMEOUT) {
// bug in AAudio pre-Q caused it to return this
result = AAUDIO_OK;
} else {
LOGD("%s() stream_waitForStateChange returned %d", __func__, result);
oboeResult = static_cast<Result>(result);
break;
}
}
if (currentAAudioState != aaudioNextState) { // state changed?
oboeResult = Result::OK;
break;
}

// Did we timeout or did user ask for non-blocking?
if (timeLeftNanos <= 0) {
break;
}

// No change yet so sleep.
mLock.unlock(); // Don't sleep while locked.
if (durationNanos > timeLeftNanos) {
durationNanos = timeLeftNanos; // last little bit
}
AudioClock::sleepForNanos(durationNanos);
timeLeftNanos -= durationNanos;
mLock.lock();
}

return (currentState != *nextState) ? Result::OK : Result::ErrorTimeout;
mLock.unlock();
return oboeResult;
}

ResultWithValue<int32_t> AudioStreamAAudio::setBufferSizeInFrames(int32_t requestedFrames) {
Expand Down
15 changes: 12 additions & 3 deletions src/common/AudioStream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,9 +75,13 @@ Result AudioStream::waitForStateTransition(StreamState startingState,
StreamState endingState,
int64_t timeoutNanoseconds)
{
StreamState state = getState();
if (state == StreamState::Closed) {
return Result::ErrorClosed;
StreamState state;
{
std::lock_guard<std::mutex> lock(mLock);
state = getState();
if (state == StreamState::Closed || state == StreamState::Disconnected) {
return Result::ErrorClosed;
}
}

StreamState nextState = state;
Expand All @@ -87,6 +91,7 @@ Result AudioStream::waitForStateTransition(StreamState startingState,
return result;
}
}

if (nextState != endingState) {
return Result::ErrorInvalidState;
} else {
Expand All @@ -98,6 +103,7 @@ Result AudioStream::start(int64_t timeoutNanoseconds)
{
Result result = requestStart();
if (result != Result::OK) return result;
if (timeoutNanoseconds <= 0) return result;
return waitForStateTransition(StreamState::Starting,
StreamState::Started, timeoutNanoseconds);
}
Expand All @@ -106,6 +112,7 @@ Result AudioStream::pause(int64_t timeoutNanoseconds)
{
Result result = requestPause();
if (result != Result::OK) return result;
if (timeoutNanoseconds <= 0) return result;
return waitForStateTransition(StreamState::Pausing,
StreamState::Paused, timeoutNanoseconds);
}
Expand All @@ -114,6 +121,7 @@ Result AudioStream::flush(int64_t timeoutNanoseconds)
{
Result result = requestFlush();
if (result != Result::OK) return result;
if (timeoutNanoseconds <= 0) return result;
return waitForStateTransition(StreamState::Flushing,
StreamState::Flushed, timeoutNanoseconds);
}
Expand All @@ -122,6 +130,7 @@ Result AudioStream::stop(int64_t timeoutNanoseconds)
{
Result result = requestStop();
if (result != Result::OK) return result;
if (timeoutNanoseconds <= 0) return result;
return waitForStateTransition(StreamState::Stopping,
StreamState::Stopped, timeoutNanoseconds);
}
Expand Down

0 comments on commit be13441

Please sign in to comment.