From 6443f4c79280e7e54fa3a4cd460c2888bd8e18df Mon Sep 17 00:00:00 2001 From: michal Date: Wed, 13 May 2026 16:55:29 +0200 Subject: [PATCH] refactor: shared_ptr instead of raw ptr in oboe --- .../android/core/AndroidAudioRecorder.cpp | 63 ++++++------------- .../android/core/AndroidAudioRecorder.h | 4 +- .../cpp/audioapi/android/core/AudioPlayer.cpp | 13 ++-- .../cpp/audioapi/android/core/AudioPlayer.h | 5 +- .../common/cpp/audioapi/core/AudioContext.cpp | 1 + 5 files changed, 33 insertions(+), 53 deletions(-) diff --git a/packages/react-native-audio-api/android/src/main/cpp/audioapi/android/core/AndroidAudioRecorder.cpp b/packages/react-native-audio-api/android/src/main/cpp/audioapi/android/core/AndroidAudioRecorder.cpp index 0d33e8423..5ab46be3c 100644 --- a/packages/react-native-audio-api/android/src/main/cpp/audioapi/android/core/AndroidAudioRecorder.cpp +++ b/packages/react-native-audio-api/android/src/main/cpp/audioapi/android/core/AndroidAudioRecorder.cpp @@ -33,49 +33,24 @@ AndroidAudioRecorder::AndroidAudioRecorder( streamMaxBufferSizeInFrames_(0) {} /// @brief Destructor ensures that the audio stream and each output type are closed and flushed up remaining data. -/// TODO: Possibly locks here are not necessary, but we might have an issue with oboe having raw pointer to the -/// recorder (and player) instances, thus creating race conditions during destruction. -/// callable from the JS thread only (i hope). +/// callable from the JS thread or hadnled by audio thread (if js dropped recorder first). AndroidAudioRecorder::~AndroidAudioRecorder() { - std::shared_ptr fileWriter; - std::shared_ptr dataCallback; - std::shared_ptr adapterNode; - { - std::scoped_lock dtorLock(callbackMutex_, fileWriterMutex_, adapterNodeMutex_); - - if (usesFileOutput()) { - fileOutputConfigured_.store(false, std::memory_order_release); - fileWriter = std::move(fileWriter_); - } - - if (usesCallback()) { - callbackOutputConfigured_.store(false, std::memory_order_release); - dataCallback = std::move(dataCallback_); - } - - if (isConnected()) { - connectedConfigured_.store(false, std::memory_order_release); - adapterNode = std::move(adapterNode_); - deinterleavingBuffer_ = nullptr; - } - - fileOutputEnabled_.store(false, std::memory_order_release); - callbackOutputEnabled_.store(false, std::memory_order_release); - isConnected_.store(false, std::memory_order_release); - } - - if (fileWriter != nullptr) { - fileWriter->closeFile(); - } - - if (dataCallback != nullptr) { - dataCallback->cleanup(); - } - - if (adapterNode != nullptr) { - adapterNode->adapterCleanup(); - } - + // there is no need to lock here, as there could be two threads that can destruct js gc and audio thread one (or one created by it) + // if we are on js: + // audio thread dropped recorder so onAudioReady callback would not be called anymore + // + // if we are on audio thread: + // js dropped recorder and oboe states that "callback object cannot be deleted before the stream is deleted" + if (fileWriter_ != nullptr) { + fileWriter_->closeFile(); + } + if (dataCallback_ != nullptr) { + dataCallback_->cleanup(); + } + if (adapterNode_ != nullptr) { + adapterNode_->adapterCleanup(); + } + // oboe could be handling stopping and closing the stream, sanity check just in case if (mStream_ != nullptr) { mStream_->requestStop(); mStream_->close(); @@ -100,8 +75,8 @@ Result AndroidAudioRecorder::openAudioStream() { ->setFormatConversionAllowed(true) ->setPerformanceMode(oboe::PerformanceMode::None) ->setSampleRateConversionQuality(oboe::SampleRateConversionQuality::Medium) - ->setDataCallback(this) - ->setErrorCallback(this); + ->setDataCallback(shared_from_this()) + ->setErrorCallback(shared_from_this()); auto result = builder.openStream(mStream_); diff --git a/packages/react-native-audio-api/android/src/main/cpp/audioapi/android/core/AndroidAudioRecorder.h b/packages/react-native-audio-api/android/src/main/cpp/audioapi/android/core/AndroidAudioRecorder.h index 2e6e6d7a3..aaf79cbd5 100644 --- a/packages/react-native-audio-api/android/src/main/cpp/audioapi/android/core/AndroidAudioRecorder.h +++ b/packages/react-native-audio-api/android/src/main/cpp/audioapi/android/core/AndroidAudioRecorder.h @@ -17,7 +17,9 @@ class AndroidRecorderCallback; class AndroidFileWriterBackend; class AudioEventHandlerRegistry; -class AndroidAudioRecorder : public oboe::AudioStreamCallback, public AudioRecorder { +class AndroidAudioRecorder : public oboe::AudioStreamCallback, + public AudioRecorder, + public std::enable_shared_from_this { public: explicit AndroidAudioRecorder( const std::shared_ptr &audioEventHandlerRegistry); diff --git a/packages/react-native-audio-api/android/src/main/cpp/audioapi/android/core/AudioPlayer.cpp b/packages/react-native-audio-api/android/src/main/cpp/audioapi/android/core/AudioPlayer.cpp index a88c634c7..4ae5ff0b9 100644 --- a/packages/react-native-audio-api/android/src/main/cpp/audioapi/android/core/AudioPlayer.cpp +++ b/packages/react-native-audio-api/android/src/main/cpp/audioapi/android/core/AudioPlayer.cpp @@ -18,9 +18,7 @@ AudioPlayer::AudioPlayer( : renderAudio_(renderAudio), sampleRate_(sampleRate), channelCount_(channelCount), - isRunning_(false) { - isInitialized_ = openAudioStream(); -} + isRunning_(false) {} bool AudioPlayer::openAudioStream() { AudioStreamBuilder builder; @@ -32,9 +30,9 @@ bool AudioPlayer::openAudioStream() { ->setChannelCount(channelCount_) ->setSampleRateConversionQuality(SampleRateConversionQuality::Medium) ->setFramesPerDataCallback(RENDER_QUANTUM_SIZE) - ->setDataCallback(this) - ->setSampleRate(static_cast(sampleRate_)) - ->setErrorCallback(this); + ->setDataCallback(shared_from_this()) + ->setErrorCallback(shared_from_this()) + ->setSampleRate(static_cast(sampleRate_)); auto result = builder.openStream(mStream_); if (result != oboe::Result::OK || mStream_ == nullptr) { @@ -44,6 +42,7 @@ bool AudioPlayer::openAudioStream() { } buffer_ = std::make_shared(RENDER_QUANTUM_SIZE, channelCount_, sampleRate_); + isInitialized_ = true; return true; } @@ -105,7 +104,7 @@ AudioPlayer::onAudioReady(AudioStream *oboeStream, void *audioData, int32_t numF return DataCallbackResult::Continue; } - auto buffer = static_cast(audioData); + auto *buffer = static_cast(audioData); int processedFrames = 0; while (processedFrames < numFrames) { diff --git a/packages/react-native-audio-api/android/src/main/cpp/audioapi/android/core/AudioPlayer.h b/packages/react-native-audio-api/android/src/main/cpp/audioapi/android/core/AudioPlayer.h index 878ce7b14..b7213bfee 100644 --- a/packages/react-native-audio-api/android/src/main/cpp/audioapi/android/core/AudioPlayer.h +++ b/packages/react-native-audio-api/android/src/main/cpp/audioapi/android/core/AudioPlayer.h @@ -14,8 +14,11 @@ using namespace oboe; class AudioContext; -class AudioPlayer : public AudioStreamDataCallback, AudioStreamErrorCallback { +class AudioPlayer : public AudioStreamDataCallback, + public AudioStreamErrorCallback, + public std::enable_shared_from_this { public: + friend class AudioContext; AudioPlayer( const std::function, int)> &renderAudio, float sampleRate, diff --git a/packages/react-native-audio-api/common/cpp/audioapi/core/AudioContext.cpp b/packages/react-native-audio-api/common/cpp/audioapi/core/AudioContext.cpp index cc7497116..56a403ba8 100644 --- a/packages/react-native-audio-api/common/cpp/audioapi/core/AudioContext.cpp +++ b/packages/react-native-audio-api/common/cpp/audioapi/core/AudioContext.cpp @@ -28,6 +28,7 @@ void AudioContext::initialize() { #ifdef ANDROID audioPlayer_ = std::make_shared( this->renderAudio(), getSampleRate(), destination_->getChannelCount()); + audioPlayer_->openAudioStream(); #else audioPlayer_ = std::make_shared( this->renderAudio(), getSampleRate(), destination_->getChannelCount());