Use shared_ptr in FilterAudioStream (#2037)

This is used when Oboe does sample rate conversion. Or format or channel count conversion.

It used to use a raw pointer, which could cause crashes.

Fixes #2035

include/oboe/AudioStream.h

@@ -54,7 +54,7 @@ class AudioStream : public AudioStreamBase {
      */
     explicit AudioStream(const AudioStreamBuilder &builder);
 
-    virtual ~AudioStream() = default;
+    virtual ~AudioStream();
 
     /**
      * Open a stream based on the current settings.

src/aaudio/AudioStreamAAudio.cpp

@@ -60,9 +60,13 @@ static aaudio_data_callback_result_t oboe_aaudio_data_callback_proc(
 // This runs in its own thread.
 // Only one of these threads will be launched from internalErrorCallback().
 // It calls app error callbacks from a static function in case the stream gets deleted.
-static void oboe_aaudio_error_thread_proc(AudioStreamAAudio *oboeStream,
+static void oboe_aaudio_error_thread_proc_common(AudioStreamAAudio *oboeStream,
                                           Result error) {
-    LOGD("%s(,%d) - entering >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>", __func__, error);
+#if 0
+    LOGE("%s() sleep for 5 seconds", __func__);
+    usleep(5*1000*1000);
+    LOGD("%s() - woke up -------------------------", __func__);
+#endif
     AudioStreamErrorCallback *errorCallback = oboeStream->getErrorCallback();
     if (errorCallback == nullptr) return; // should be impossible
     bool isErrorHandled = errorCallback->onError(oboeStream, error);
@@ -74,16 +78,24 @@ static void oboe_aaudio_error_thread_proc(AudioStreamAAudio *oboeStream,
         // Warning, oboeStream may get deleted by this callback.
         errorCallback->onErrorAfterClose(oboeStream, error);
     }
+}
+
+// Callback thread for raw pointers.
+static void oboe_aaudio_error_thread_proc(AudioStreamAAudio *oboeStream,
+                                          Result error) {
+    LOGD("%s(,%d) - entering >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>", __func__, error);
+    oboe_aaudio_error_thread_proc_common(oboeStream, error);
     LOGD("%s() - exiting <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<", __func__);
 }
 
-// This runs in its own thread.
-// Only one of these threads will be launched from internalErrorCallback().
-// Prevents deletion of the stream if the app is using AudioStreamBuilder::openSharedStream()
+// Callback thread for shared pointers.
 static void oboe_aaudio_error_thread_proc_shared(std::shared_ptr<AudioStream> sharedStream,
                                           Result error) {
+    LOGD("%s(,%d) - entering >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>", __func__, error);
+    // Hold the shared pointer while we use the raw pointer.
     AudioStreamAAudio *oboeStream = reinterpret_cast<AudioStreamAAudio*>(sharedStream.get());
-    oboe_aaudio_error_thread_proc(oboeStream, error);
+    oboe_aaudio_error_thread_proc_common(oboeStream, error);
+    LOGD("%s() - exiting <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<", __func__);
 }
 
 namespace oboe {

src/common/AudioStream.cpp

@@ -30,6 +30,12 @@ namespace oboe {
  */
 AudioStream::AudioStream(const AudioStreamBuilder &builder)
         : AudioStreamBase(builder) {
+    LOGD("Constructor for AudioStream at %p", this);
+}
+
+AudioStream::~AudioStream() {
+    // This is to help debug use after free bugs.
+    LOGD("Destructor for AudioStream at %p", this);
 }
 
 Result AudioStream::close() {

src/common/AudioStreamBuilder.cpp

@@ -117,7 +117,7 @@ Result AudioStreamBuilder::openStreamInternal(AudioStream **streamPP) {
     // Do we need to make a child stream and convert.
     if (conversionNeeded) {
         AudioStream *tempStream;
-        result = childBuilder.openStream(&tempStream);
+        result = childBuilder.openStreamInternal(&tempStream);
         if (result != Result::OK) {
             return result;
         }
@@ -144,7 +144,9 @@ Result AudioStreamBuilder::openStreamInternal(AudioStream **streamPP) {
 
             // Use childStream in a FilterAudioStream.
             LOGI("%s() create a FilterAudioStream for data conversion.", __func__);
-            FilterAudioStream *filterStream = new FilterAudioStream(parentBuilder, tempStream);
+            std::shared_ptr<AudioStream> childStream(tempStream);
+            FilterAudioStream *filterStream = new FilterAudioStream(parentBuilder, childStream);
+            childStream->setWeakThis(childStream);
             result = filterStream->configureFlowGraph();
             if (result !=  Result::OK) {
                 filterStream->close();

src/common/FilterAudioStream.h

@@ -38,9 +38,9 @@ class FilterAudioStream : public AudioStream, AudioStreamCallback {
      *
      * @param builder containing all the stream's attributes
      */
-    FilterAudioStream(const AudioStreamBuilder &builder, AudioStream *childStream)
+    FilterAudioStream(const AudioStreamBuilder &builder, std::shared_ptr<AudioStream> childStream)
     : AudioStream(builder)
-    , mChildStream(childStream) {
+     , mChildStream(childStream) {
         // Intercept the callback if used.
         if (builder.isErrorCallbackSpecified()) {
             mErrorCallback = mChildStream->swapErrorCallback(this);
@@ -66,10 +66,6 @@ class FilterAudioStream : public AudioStream, AudioStreamCallback {
 
     virtual ~FilterAudioStream() = default;
 
-    AudioStream *getChildStream() const {
-        return mChildStream.get();
-    }
-
     Result configureFlowGraph();
 
     // Close child and parent.
@@ -216,7 +212,7 @@ class FilterAudioStream : public AudioStream, AudioStreamCallback {
 
 private:
 
-    std::unique_ptr<AudioStream>             mChildStream; // this stream wraps the child stream
+    std::shared_ptr<AudioStream>             mChildStream; // this stream wraps the child stream
     std::unique_ptr<DataConversionFlowGraph> mFlowGraph; // for converting data
     std::unique_ptr<uint8_t[]>               mBlockingBuffer; // temp buffer for write()
     double                                   mRateScaler = 1.0; // ratio parent/child sample rates