oboe: use shared_ptr for callbacks

apps/OboeTester/app/src/main/cpp/TestErrorCallback.cpp

@@ -23,15 +23,20 @@ using namespace oboe;
 
 oboe::Result TestErrorCallback::open() {
     mCallbackMagic = 0;
-    mDataCallback = std::make_unique<MyDataCallback>();
-    mErrorCallback = std::make_unique<MyErrorCallback>(this);
+    mDataCallback = std::make_shared<MyDataCallback>();
+    mErrorCallback = std::make_shared<MyErrorCallback>(this);
     AudioStreamBuilder builder;
     oboe::Result result = builder.setSharingMode(oboe::SharingMode::Exclusive)
             ->setPerformanceMode(oboe::PerformanceMode::LowLatency)
             ->setFormat(oboe::AudioFormat::Float)
             ->setChannelCount(kChannelCount)
+#if 0
             ->setDataCallback(mDataCallback.get())
-            ->setErrorCallback(mErrorCallback.get())
+            ->setErrorCallback(mErrorCallback.get()) // This can lead to a crash or FAIL.
+#else
+            ->setDataCallback(mDataCallback)
+            ->setErrorCallback(mErrorCallback) // shared_ptr avoids a crash
+#endif
             ->openStream(mStream);
     return result;
 }

apps/OboeTester/app/src/main/cpp/TestErrorCallback.h

@@ -44,7 +44,9 @@ class TestErrorCallback {
 private:
 
     void cleanup() {
+        mDataCallback.reset();
         mErrorCallback.reset();
+        mStream.reset();
     }
 
     class MyDataCallback : public oboe::AudioStreamDataCallback {    public:
@@ -72,11 +74,11 @@ class TestErrorCallback {
         void onErrorBeforeClose(oboe::AudioStream *oboeStream, oboe::Result error) override {
             LOGE("%s() - error = %s, parent = %p",
                  __func__, oboe::convertToText(error), &mParent);
-            // Trigger a crash by deleting this callback object while in use!
+            // Trigger a crash by "deleting" this callback object while in use!
             // Do not try this at home. We are just trying to reproduce the crash
             // reported in #1603.
             std::thread t([this]() {
-                    this->mParent->cleanup();
+                    this->mParent->cleanup(); // Possibly delete stream and callback objects.
                     LOGE("onErrorBeforeClose called cleanup!");
                 });
             t.detach();
@@ -102,8 +104,8 @@ class TestErrorCallback {
     };
 
     std::shared_ptr<oboe::AudioStream> mStream;
-    std::unique_ptr<MyDataCallback> mDataCallback;
-    std::unique_ptr<MyErrorCallback> mErrorCallback;
+    std::shared_ptr<MyDataCallback> mDataCallback;
+    std::shared_ptr<MyErrorCallback> mErrorCallback;
 
     static constexpr int kChannelCount = 2;
 };

include/oboe/AudioStreamBase.h

@@ -188,9 +188,11 @@ class AudioStreamBase {
 protected:
     /** The callback which will be fired when new data is ready to be read/written. **/
     AudioStreamDataCallback        *mDataCallback = nullptr;
+    std::shared_ptr<AudioStreamDataCallback> mSharedDataCallback;
 
     /** The callback which will be fired when an error or a disconnect occurs. **/
     AudioStreamErrorCallback       *mErrorCallback = nullptr;
+    std::shared_ptr<AudioStreamErrorCallback> mSharedErrorCallback;
 
     /** Number of audio frames which will be requested in each callback */
     int32_t                         mFramesPerCallback = kUnspecified;

include/oboe/AudioStreamBuilder.h

@@ -354,11 +354,31 @@ class AudioStreamBuilder : public AudioStreamBase {
      * <strong>Important: See AudioStreamCallback for restrictions on what may be called
      * from the callback methods.</strong>
      *
+     * We pass a shared_ptr so that the sharedDataCallback object cannot be deleted
+     * before the stream is deleted.
+     *
      * @param dataCallback
      * @return pointer to the builder so calls can be chained
      */
-    AudioStreamBuilder *setDataCallback(oboe::AudioStreamDataCallback *dataCallback) {
+    AudioStreamBuilder *setDataCallback(std::shared_ptr<AudioStreamDataCallback> sharedDataCallback) {
+        // Use this raw pointer in the rest of the code to retain backwards compatibility.
+        mDataCallback = sharedDataCallback.get();
+        // Hold a shared_ptr to protect the raw pointer for the lifetime of the stream.
+        mSharedDataCallback = sharedDataCallback;
+        return this;
+    }
+
+    /**
+    * Pass a raw pointer to a data callback. This is not recommended because the dataCallback
+    * object might get deleted by the app while it is being used.
+    *
+    * @deprecated Call setDataCallback(std::shared_ptr<AudioStreamDataCallback>) instead.
+    * @param dataCallback
+    * @return pointer to the builder so calls can be chained
+    */
+    AudioStreamBuilder *setDataCallback(AudioStreamDataCallback *dataCallback) {
         mDataCallback = dataCallback;
+        mSharedDataCallback = nullptr;
         return this;
     }
 
@@ -374,11 +394,32 @@ class AudioStreamBuilder : public AudioStreamBase {
      * <strong>When an error callback occurs, the associated stream must be stopped and closed
      * in a separate thread.</strong>
      *
-     * @param errorCallback
+     * We pass a shared_ptr so that the errorCallback object cannot be deleted before the stream is deleted.
+     * If the stream was created using a shared_ptr then the stream cannot be deleted before the
+     * error callback has finished running.
+     *
+     * @param sharedErrorCallback
      * @return pointer to the builder so calls can be chained
      */
-    AudioStreamBuilder *setErrorCallback(oboe::AudioStreamErrorCallback *errorCallback) {
+    AudioStreamBuilder *setErrorCallback(std::shared_ptr<AudioStreamErrorCallback> sharedErrorCallback) {
+        // Use this raw pointer in the rest of the code to retain backwards compatibility.
+        mErrorCallback = sharedErrorCallback.get();
+        // Hold a shared_ptr to protect the raw pointer for the lifetime of the stream.
+        mSharedErrorCallback = sharedErrorCallback;
+        return this;
+    }
+
+    /**
+    * Pass a raw pointer to an error callback. This is not recommended because the errorCallback
+    * object might get deleted by the app while it is being used.
+    *
+    * @deprecated Call setErrorCallback(std::shared_ptr<AudioStreamErrorCallback>) instead.
+    * @param errorCallback
+    * @return pointer to the builder so calls can be chained
+    */
+    AudioStreamBuilder *setErrorCallback(AudioStreamErrorCallback *errorCallback) {
         mErrorCallback = errorCallback;
+        mSharedErrorCallback = nullptr;
         return this;
     }