Crash when headset plugs in/out #406
Comments
|
What version of Oboe are you using? I think that may have been fixed in this Pull Request: Please let me know if that fixes the problem. It is probably worth releasing a new stable version with that fix. |
|
Hi Phil, thanks for the quick reply. I just tested with master and it still crashes.
To me it sounds like Please correct me if that doesn't sound right. |
|
I just realized that close already locks a mutex. Result AudioStreamAAudio::waitForStateChange(StreamState currentState,
StreamState *nextState,
int64_t timeoutNanoseconds) {
std::lock_guard<std::mutex> lock(mLock); // <- but now it shouldn't be used in time critical code anymore
AAudioStream *stream = mAAudioStream.load();
if (stream != nullptr) {
... |
|
@pokey909 - I appreciate your help with this Issue. We want to get it right. You wrote:
Oh! Are you using a data callback? I was thinking you were using read/write. Then yes, the synchronous start() method does have a problem. The wait method starts running while the stream still exists. Then the stream gets deleted while it is still waiting. Not good.
I agree. We will probably deprecate it. Calling requestStart() is better. The waitForStateChange() method is dangerous when combined with the asynchronous close in the error callback. I wish we could use recursive locks. Thanks for reporting this. |
|
I don't think recursive mutexes are needed. |
|
Actions:
|
|
So what is the suggested way to wait for a state change then? |
Generally, one does not need to call waitForStateChange(). When starting you can just call requestStart(). You don't really need to wait for the internal state change. One time you might want to waitForStateChange() is when doing a flush and you want to make sure the stream is PAUSED. But the stream and callbacks are not running when paused so it is safe to call waitForStateChange(). Maybe the wait should be moved to the beginning of the flush() method. |
|
There may be a way to make waitForStateChange() safer. The problem is that we call into AAudioStream_waitForStateChange() and sleep in that routine. We don't want to sleep with the lock. So we could instead just sleep in our Oboe method and call AAudioStream_waitForStateChange() with a timeout=0. That way the AAudio function will not block. So we could safely surround that with a lock. Then unlock around the sleep in Oboe. Then the Oboe waitForStateChange() would be synchronized with the close and would be safe. |
|
Thanks for clarifying Phil. |
Call AAudio with timeout zero to avoid blocking. Prevent crashes if close() called from another thread. Possible fix for #406
Do not return ErrorTimeout is user passed timeout==0. Related to #406
|
@pokey909 The current master might fix this. It now uses locks in waitForStateChange() in a way that will not block other threads. |
|
@philburk Alex is on vacation - so I will retest the app with the latest changes and report back. |
|
@philburk We are not able to reproduce the issue anymore with the current master. |
|
Thanks. That is great news. I will close this Issue. |
The crash happens when the stream is in Paused state and a headset gets connected or disconnected.
Next time you call
play()(the synchronous API) we get the crash.And the reason seems to be that as soon as the stream starts again, it will call
onErrorInThreadon the stream since it is now inDISCONNECTEDstate and delegate toonErrorAfterClosefunctions which run in their own detached thread.We restart the stream in the callbacks but the stream will be closed. Concurrently, the
start()function will runwaitForStateChangewhich is not supposed to be called during or afterclose(). And here the crash happens since the underlying stream got deleted.I can work around that problem by only using the
requestStart()function since it doesn't callwaitForStateTransition.Are we not supposed to use the synchronous methods at all? I don't see a way to make that threadsafe unless we use locks to synchronize the detached thread from
oboe_aaudio_error_callback_procwith thewaitForStateTransitionmethod.Stacktrace below:
The text was updated successfully, but these errors were encountered: