diff --git a/media/jni/soundpool/android_media_SoundPool.cpp b/media/jni/soundpool/android_media_SoundPool.cpp index a66d99fbd9f4..66870d384f08 100644 --- a/media/jni/soundpool/android_media_SoundPool.cpp +++ b/media/jni/soundpool/android_media_SoundPool.cpp @@ -33,10 +33,156 @@ static struct fields_t { jmethodID mPostEvent; jclass mSoundPoolClass; } fields; -static inline SoundPool* MusterSoundPool(JNIEnv *env, jobject thiz) { - // NOLINTNEXTLINE(performance-no-int-to-ptr) - return reinterpret_cast(env->GetLongField(thiz, fields.mNativeContext)); + +namespace { + +/** + * ObjectManager creates a native "object" on the heap and stores + * its pointer in a long field in a Java object. + * + * The type T must have 3 properties in the current implementation. + * 1) A T{} default constructor which represents a nullValue. + * 2) T::operator bool() const efficient detection of such a nullValue. + * 3) T must be copyable. + * + * Some examples of such a type T are std::shared_ptr<>, android::sp<>, + * std::optional, std::function<>, etc. + * + * Using set() with a nullValue T results in destroying the underlying native + * "object" if it exists. A nullValue T is returned by get() if there is + * no underlying native Object. + * + * This class is thread safe for multiple access. + * + * Design notes: + * 1) For objects of type T that do not naturally have an "nullValue", + * wrapping with + * a) TOpt, where TOpt = std::optional + * b) TShared, where TShared = std::shared_ptr + * + * 2) An overload for an explicit equality comparable nullValue such as + * get(..., const T& nullValue) or set(..., const T& nullValue) + * is omitted. An alternative is to pass a fixed nullValue in the constructor. + */ +template +class ObjectManager +{ +// Can a jlong hold a pointer? +static_assert(sizeof(jlong) >= sizeof(void*)); + +public: + // fieldId is associated with a Java long member variable in the object. + // ObjectManager will store the native pointer in that field. + // + // If a native object is set() in that field, it + explicit ObjectManager(jfieldID fieldId) : mFieldId(fieldId) {} + ~ObjectManager() { + ALOGE_IF(mObjectCount != 0, "%s: mObjectCount: %d should be zero on destruction", + __func__, mObjectCount.load()); + // Design note: it would be possible to keep a map of the outstanding allocated + // objects and force a delete on them on ObjectManager destruction. + // The consequences of that is probably worse than keeping them alive. + } + + // Retrieves the associated object, returns nullValue T if not available. + T get(JNIEnv *env, jobject thiz) { + std::lock_guard lg(mLock); + // NOLINTNEXTLINE(performance-no-int-to-ptr) + auto ptr = reinterpret_cast(env->GetLongField(thiz, mFieldId)); + if (ptr != nullptr) { + return *ptr; + } + return {}; + } + + // Sets the object and returns the old one. + // + // If the old object doesn't exist, then nullValue T is returned. + // If the new object is false by operator bool(), the internal object is destroyed. + // Note: The old object is returned so if T is a smart pointer, it can be held + // by the caller to be deleted outside of any external lock. + // + // Remember to call set(env, thiz, {}) to destroy the object in the Java + // object finalize to avoid orphaned objects on the heap. + T set(JNIEnv *env, jobject thiz, const T& newObject) { + std::lock_guard lg(mLock); + // NOLINTNEXTLINE(performance-no-int-to-ptr) + auto ptr = reinterpret_cast(env->GetLongField(thiz, mFieldId)); + if (ptr != nullptr) { + T old = std::move(*ptr); // *ptr will be replaced or deleted. + if (newObject) { + env->SetLongField(thiz, mFieldId, (jlong)0); + delete ptr; + --mObjectCount; + } else { + *ptr = newObject; + } + return old; + } else { + if (newObject) { + env->SetLongField(thiz, mFieldId, (jlong)new T(newObject)); + ++mObjectCount; + } + return {}; + } + } + + // Returns the number of outstanding objects. + // + // This is purely for debugging purposes and tracks the number of active Java + // objects that have native T objects; hence represents the number of + // T heap allocations we have made. + // + // When all those Java objects have been finalized we expect this to go to 0. + int32_t getObjectCount() const { + return mObjectCount; + } + +private: + // NOLINTNEXTLINE(misc-misplaced-const) + const jfieldID mFieldId; // '_jfieldID *const' + + // mObjectCount is the number of outstanding native T heap allocations we have + // made (and thus the number of active Java objects which are associated with them). + std::atomic_int32_t mObjectCount{}; + + mutable std::mutex mLock; +}; + +// We use SoundPoolManager to associate a native std::shared_ptr +// object with a field in the Java object. +// +// We can then retrieve the std::shared_ptr from the object. +// +// Design notes: +// 1) This is based on ObjectManager class. +// 2) An alternative that does not require a field in the Java object +// is to create an associative map using as a key a NewWeakGlobalRef +// to the Java object. +// The problem of this method is that lookup is O(N) because comparison +// between the WeakGlobalRef to a JNI jobject LocalRef must be done +// through the JNI IsSameObject() call, hence iterative through the map. +// One advantage of this method is that manual garbage collection +// is possible by checking if the WeakGlobalRef is null equivalent. + +auto& getSoundPoolManager() { + static ObjectManager> soundPoolManager(fields.mNativeContext); + return soundPoolManager; } + +inline auto getSoundPool(JNIEnv *env, jobject thiz) { + return getSoundPoolManager().get(env, thiz); +} + +// Note: one must call setSoundPool(env, thiz, nullptr) to release any native resources +// somewhere in the Java object finalize(). +inline auto setSoundPool( + JNIEnv *env, jobject thiz, const std::shared_ptr& soundPool) { + return getSoundPoolManager().set(env, thiz, soundPool); +} + +} // namespace + static const char* const kAudioAttributesClassPathName = "android/media/AudioAttributes"; struct audio_attributes_fields_t { jfieldID fieldUsage; // AudioAttributes.mUsage @@ -53,18 +199,18 @@ android_media_SoundPool_load_FD(JNIEnv *env, jobject thiz, jobject fileDescripto jlong offset, jlong length, jint priority) { ALOGV("android_media_SoundPool_load_FD"); - SoundPool *ap = MusterSoundPool(env, thiz); - if (ap == nullptr) return 0; - return (jint) ap->load(jniGetFDFromFileDescriptor(env, fileDescriptor), + auto soundPool = getSoundPool(env, thiz); + if (soundPool == nullptr) return 0; + return (jint) soundPool->load(jniGetFDFromFileDescriptor(env, fileDescriptor), int64_t(offset), int64_t(length), int(priority)); } static jboolean android_media_SoundPool_unload(JNIEnv *env, jobject thiz, jint sampleID) { ALOGV("android_media_SoundPool_unload\n"); - SoundPool *ap = MusterSoundPool(env, thiz); - if (ap == nullptr) return JNI_FALSE; - return ap->unload(sampleID) ? JNI_TRUE : JNI_FALSE; + auto soundPool = getSoundPool(env, thiz); + if (soundPool == nullptr) return JNI_FALSE; + return soundPool->unload(sampleID) ? JNI_TRUE : JNI_FALSE; } static jint @@ -73,54 +219,54 @@ android_media_SoundPool_play(JNIEnv *env, jobject thiz, jint sampleID, jfloat rate) { ALOGV("android_media_SoundPool_play\n"); - SoundPool *ap = MusterSoundPool(env, thiz); - if (ap == nullptr) return 0; - return (jint) ap->play(sampleID, leftVolume, rightVolume, priority, loop, rate); + auto soundPool = getSoundPool(env, thiz); + if (soundPool == nullptr) return 0; + return (jint) soundPool->play(sampleID, leftVolume, rightVolume, priority, loop, rate); } static void android_media_SoundPool_pause(JNIEnv *env, jobject thiz, jint channelID) { ALOGV("android_media_SoundPool_pause"); - SoundPool *ap = MusterSoundPool(env, thiz); - if (ap == nullptr) return; - ap->pause(channelID); + auto soundPool = getSoundPool(env, thiz); + if (soundPool == nullptr) return; + soundPool->pause(channelID); } static void android_media_SoundPool_resume(JNIEnv *env, jobject thiz, jint channelID) { ALOGV("android_media_SoundPool_resume"); - SoundPool *ap = MusterSoundPool(env, thiz); - if (ap == nullptr) return; - ap->resume(channelID); + auto soundPool = getSoundPool(env, thiz); + if (soundPool == nullptr) return; + soundPool->resume(channelID); } static void android_media_SoundPool_autoPause(JNIEnv *env, jobject thiz) { ALOGV("android_media_SoundPool_autoPause"); - SoundPool *ap = MusterSoundPool(env, thiz); - if (ap == nullptr) return; - ap->autoPause(); + auto soundPool = getSoundPool(env, thiz); + if (soundPool == nullptr) return; + soundPool->autoPause(); } static void android_media_SoundPool_autoResume(JNIEnv *env, jobject thiz) { ALOGV("android_media_SoundPool_autoResume"); - SoundPool *ap = MusterSoundPool(env, thiz); - if (ap == nullptr) return; - ap->autoResume(); + auto soundPool = getSoundPool(env, thiz); + if (soundPool == nullptr) return; + soundPool->autoResume(); } static void android_media_SoundPool_stop(JNIEnv *env, jobject thiz, jint channelID) { ALOGV("android_media_SoundPool_stop"); - SoundPool *ap = MusterSoundPool(env, thiz); - if (ap == nullptr) return; - ap->stop(channelID); + auto soundPool = getSoundPool(env, thiz); + if (soundPool == nullptr) return; + soundPool->stop(channelID); } static void @@ -128,18 +274,18 @@ android_media_SoundPool_setVolume(JNIEnv *env, jobject thiz, jint channelID, jfloat leftVolume, jfloat rightVolume) { ALOGV("android_media_SoundPool_setVolume"); - SoundPool *ap = MusterSoundPool(env, thiz); - if (ap == nullptr) return; - ap->setVolume(channelID, (float) leftVolume, (float) rightVolume); + auto soundPool = getSoundPool(env, thiz); + if (soundPool == nullptr) return; + soundPool->setVolume(channelID, (float) leftVolume, (float) rightVolume); } static void android_media_SoundPool_mute(JNIEnv *env, jobject thiz, jboolean muting) { ALOGV("android_media_SoundPool_mute(%d)", muting); - SoundPool *ap = MusterSoundPool(env, thiz); - if (ap == nullptr) return; - ap->mute(muting == JNI_TRUE); + auto soundPool = getSoundPool(env, thiz); + if (soundPool == nullptr) return; + soundPool->mute(muting == JNI_TRUE); } static void @@ -147,9 +293,9 @@ android_media_SoundPool_setPriority(JNIEnv *env, jobject thiz, jint channelID, jint priority) { ALOGV("android_media_SoundPool_setPriority"); - SoundPool *ap = MusterSoundPool(env, thiz); - if (ap == nullptr) return; - ap->setPriority(channelID, (int) priority); + auto soundPool = getSoundPool(env, thiz); + if (soundPool == nullptr) return; + soundPool->setPriority(channelID, (int) priority); } static void @@ -157,9 +303,9 @@ android_media_SoundPool_setLoop(JNIEnv *env, jobject thiz, jint channelID, int loop) { ALOGV("android_media_SoundPool_setLoop"); - SoundPool *ap = MusterSoundPool(env, thiz); - if (ap == nullptr) return; - ap->setLoop(channelID, loop); + auto soundPool = getSoundPool(env, thiz); + if (soundPool == nullptr) return; + soundPool->setLoop(channelID, loop); } static void @@ -167,9 +313,9 @@ android_media_SoundPool_setRate(JNIEnv *env, jobject thiz, jint channelID, jfloat rate) { ALOGV("android_media_SoundPool_setRate"); - SoundPool *ap = MusterSoundPool(env, thiz); - if (ap == nullptr) return; - ap->setRate(channelID, (float) rate); + auto soundPool = getSoundPool(env, thiz); + if (soundPool == nullptr) return; + soundPool->setRate(channelID, (float) rate); } static void android_media_callback(SoundPoolEvent event, SoundPool* soundPool, void* user) @@ -206,17 +352,16 @@ android_media_SoundPool_native_setup(JNIEnv *env, jobject thiz, jobject weakRef, ALOGV("android_media_SoundPool_native_setup"); ScopedUtfChars opPackageNameStr(env, opPackageName); - auto *ap = new SoundPool(maxChannels, paa, opPackageNameStr.c_str()); - if (ap == nullptr) { - return -1; - } - - // save pointer to SoundPool C++ object in opaque field in Java object - env->SetLongField(thiz, fields.mNativeContext, (jlong) ap); + auto soundPool = std::make_shared(maxChannels, paa, opPackageNameStr.c_str()); // set callback with weak reference jobject globalWeakRef = env->NewGlobalRef(weakRef); - ap->setCallback(android_media_callback, globalWeakRef); + soundPool->setCallback(android_media_callback, globalWeakRef); + + // register with SoundPoolManager. + auto oldSoundPool = setSoundPool(env, thiz, soundPool); + ALOGW_IF(oldSoundPool != nullptr, "%s: Aliased SoundPool object %p", + __func__, oldSoundPool.get()); // audio attributes were copied in SoundPool creation free(paa); @@ -228,20 +373,23 @@ static void android_media_SoundPool_release(JNIEnv *env, jobject thiz) { ALOGV("android_media_SoundPool_release"); - SoundPool *ap = MusterSoundPool(env, thiz); - if (ap != nullptr) { + // Remove us from SoundPoolManager. + auto oldSoundPool = setSoundPool(env, thiz, nullptr); + + // Caution: Deleting the weakRef is not race free from invoking + // the Java callback because we may not have the last remaining + // reference to the SoundPool object - another method could still + // be in progress. + if (oldSoundPool != nullptr) { // release weak reference and clear callback - auto weakRef = (jobject) ap->getUserData(); - ap->setCallback(nullptr /* callback */, nullptr /* user */); + auto weakRef = (jobject) oldSoundPool->getUserData(); + oldSoundPool->setCallback(nullptr /* callback */, nullptr /* user */); if (weakRef != nullptr) { env->DeleteGlobalRef(weakRef); } - - // clear native context - env->SetLongField(thiz, fields.mNativeContext, 0); - delete ap; } + // destructor to oldSoundPool should occur at exit. } // ----------------------------------------------------------------------------