The client receives the callback onErrorAfterClose from oboe even if the audio stream is closed. [ Same as Issue 1578]

[FeiWang888]

It is the same as Issue 1578, #1578, which is closed, but I don't get the right answer.
To draw attention, I am not able to reopen the previous one so open a new one. Please reopen 1578 or give a comment here. Thanks.

Android version(s):
Android 8.1+
Android device(s):
Samsung Galaxy Note10+, Samsung Galaxy S21 Ultra 5G, Google Pixel 5, Samsung Galaxy A70, Samsung Galaxy A72, etc..

Oboe version:
OboeVersion1.5.0

App name used for testing:
Cisco WebexMeeting App
https://play.google.com/store/apps/details?id=com.cisco.webex.meetings&hl=en_US&gl=US

Short description
We saw a lot of crash issues reported from the Google Console and receives some client logs for that.
However, it is very hard to reproduce the issue locally
Please give advice about the root causes and how to prevent them.

The key question is why the client receives the callback onErrorAfterClose from oboe even if the audio stream is closed.
Client log:
// stop the stream (thread id 2079)
NATIVE_WME 0:0 07/18/2022 01:46:59 737 I [TID:2079][NATIVE_TID:18036][AudioEngine] [CallID=33558529]WbxAndroidAudioPlaybackNative::StopStream end, stream = 0xb400007afccefd80, stream->getState() = 10,this=0xb400007a27418018
// then close the stream (thread id 2079)
NATIVE_WME 0:0 07/18/2022 01:46:59 778 I [TID:2079][NATIVE_TID:18036][AudioEngine] [CallID=33558529]WbxAndroidAudioPlaybackNative::CloseStream end, stream = 0xb400007afccefd80, stream->getState() = 12,this=0xb400007a27418018
// deconstruct the object (thread id 2079)
NATIVE_WME 0:0 07/18/2022 01:46:59 780 I [TID:2079][NATIVE_TID:18036][AudioEngine] [CallID=33558529]WbxAndroidAudioPlaybackNative::~WbxAndroidAudioPlaybackNative,this=0xb400007a27418018
// receives the callback onErrorAfterClose from oboe (thread id 22077)
NATIVE_WME 0:0 07/18/2022 01:46:59 791 I [TID:2210][NATIVE_TID:22077][AudioEngine] [CallID=33558529]WbxAndroidAudioPlaybackNative::onErrorAfterClose, audioStream = 0xb400007afccefd80, error = ErrorDisconnected,this=0xb400007a27418018
// then the crash happens on WbxAndroidAudioPlaybackNative::onErrorAfterClose since the WbxAndroidAudioPlaybackNative object has been destroyed.

CRASH ISSUE:

signal 11 (SIGSEGV), code 1 (SEGV_MAPERR)
WbxAndroidAudioPlaybackNative::RestartStreamAfterClose(int)

pid: 0, tid: 0 >>> com.cisco.webex.meetings <<<

backtrace:
#00 pc 00000000000d6f4c /data/app/~~ikG_Vy5Itp7JxfLlKVZwdg==/com.cisco.webex.meetings-d7eqCeNia7yKgcVzOSTtuA==/lib/arm64/libaudioengine.so (WbxAndroidAudioPlaybackNative::RestartStreamAfterClose(int)+700)
#00 pc 00000000000d7abc /data/app/~~ikG_Vy5Itp7JxfLlKVZwdg==/com.cisco.webex.meetings-d7eqCeNia7yKgcVzOSTtuA==/lib/arm64/libaudioengine.so (WbxAndroidAudioPlaybackNative::onErrorAfterClose(oboe::AudioStream*, oboe::Result)+252)
#00 pc 000000000001c80c /data/app/~~ikG_Vy5Itp7JxfLlKVZwdg==/com.cisco.webex.meetings-d7eqCeNia7yKgcVzOSTtuA==/lib/arm64/liboboe.so (void* std::__ndk1::__thread_proxy<std::__ndk1::tuple<std::__ndk1::unique_ptr<std::__ndk1::__thread_struct, std::__ndk1::default_deletestd::__ndk1::__thread_struct >, void ()(std::__ndk1::shared_ptroboe::AudioStream, oboe::Result), std::__ndk1::shared_ptroboe::AudioStream, oboe::Result> >(void)+80)
#00 pc 00000000000b0048 /apex/com.android.runtime/lib64/bionic/libc.so (__pthread_start(void*)+64)
#00 pc 00000000000503c8 /apex/com.android.runtime/lib64/bionic/libc.so (__start_thread+64)

---

[philburk]

@FeiWang888 - Thanks for explaining this.

It sounds like a race condition. Here is a possible sequence of events.

1. APP: registers an error callback using WbxAndroidAudioPlaybackNative, which implements AudioStreamErrorCallback
   and also has some other methods for controlling streams.
2. USER: disconnects headphone
3. OBOE: launches threads that start the error callback process, it can take a while
4. APP: receives broadcast that the headset was disconnected and closes the stream and deletes the WbxAndroidAudioPlaybackNative object, which is still being used by the error callback thread
5. OBOE: calls WbxAndroidAudioPlaybackNative::onErrorAfterClose on deleted object and crashes

Does that sound like what you are seeing?

One problem is that we allow an app to register a callback that may get deleted by the app before it is called. We should probably require that a shared_ptr be passed in. We will review the Oboe API. We may want to add a method that will join any active callbacks.

You asked how an error callback could occur even after the stream was closed. We have seen some cases where callback threads were running and were not joined properly by the close. But in this case I think the thread was probably launched by AAudio and Oboe before your app closed the stream.

For now, I suggest not deleting the WbxAndroidAudioPlaybackNative object until your app is ready to exit.

By the way, are you opening the Oboe stream using a shared_ptr as described here?
#1578 (comment)

[philburk]

@FeiWang888 - I also recommend using Oboe 1.6.1.

What is the latest version of Android where you have seen this happen?

[robertwu1]

See the comments in AudioStreamBuilder.setCallback

     * Specifies an object to handle data or error related callbacks from the underlying API.
     *
     * This is the equivalent of calling both setDataCallback() and setErrorCallback().
     *
     * <strong>Important: See AudioStreamCallback for restrictions on what may be called
     * from the callback methods.</strong>
     *
     * When an error callback occurs, the associated stream will be stopped and closed in a separate thread.
     *
     * A note on why the streamCallback parameter is a raw pointer rather than a smart pointer:
     *
     * The caller should retain ownership of the object streamCallback points to. At first glance weak_ptr may seem like
     * a good candidate for streamCallback as this implies temporary ownership. However, a weak_ptr can only be created
     * from a shared_ptr. A shared_ptr incurs some performance overhead. The callback object is likely to be accessed
     * every few milliseconds when the stream requires new data so this overhead is something we want to avoid.
     *
     * This leaves a raw pointer as the logical type choice. The only caveat being that the caller must not destroy
     * the callback before the stream has been closed.

[philburk]

The overhead is definitely an issue for data callbacks. The original Oboe callback combined data and error callbacks. We later split them. So I think we could accept the extra overhead of shared_ptr for the separate error callbacks. Also the error callback is run in a separate thread and is more prone to race conditions.

[FeiWang888]

@FeiWang888 - I also recommend using Oboe 1.6.1.

What is the latest version of Android where you have seen this happen?

The Oboe version we use is OboeVersion1.5.0.
Does Oboe1.6.1 has the protection for this case?

[FeiWang888]

Thanks for reviewing this issue again. Here are some feedbacks. @philburk

1. According to the log, it is a possible sequence of events you mentioned.

2. "You asked how an error callback could occur even after the stream was closed. We have seen some cases where callback threads were running and were not joined properly by the close. But in this case I think the thread was probably launched by AAudio and Oboe before your app closed the stream."
   -- Is it better if the thread checks the audio stream state before sending the error callback?

3. "For now, I suggest not deleting the WbxAndroidAudioPlaybackNative object until your app is ready to exit."
   -- We will consider your suggestion.

4. We do use a shared_ptr for the oboe::AudioStream object.
   std::shared_ptroboe::AudioStream m_PlaybackStream;
   oboe::Result result = builder.openStream(m_PlaybackStream);

[FeiWang888]

@philburk
I recommend that the thread checks the audio stream state before sending the error callback.

using shared_ptr for callbacks does not guarantee the instance for callbacks will not destroy.
Example:
Person* person = new Person("frank");
std::shared_ptr shared_ptr_person(person);
person->printName();
shared_ptr_person->printName();
delete person;
shared_ptr_person->printName();

//print out
name is frank
name is frank
name is \365\357\277\367���.���������������������������������������\240\303 .....

[philburk]

using shared_ptr for callbacks does not guarantee the instance for callbacks will not destroy.

It is always possible for an app to do something very wrong. Just don't do that. You should never "delete" the raw pointer to an object that is in a shared_ptr.

[FeiWang888]

I understand your point about the pointer.
However, in this case, it will make sense if the audio stream is in the closed state, there is no reason to invoke the callback onErrorAfterClose from the oboe library. What do you think?