Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 48c07950 authored by Andy Hung's avatar Andy Hung
Browse files

SoundPool: Fix lifetime of SoundPool native object

Avoids race between release and Binder volume adjustment.

Test: atest SoundPoolAacTest
Test: atest SoundPoolHapticTest
Test: atest SoundPoolMidiTest
Test: atest SoundPoolOggTest
Test: atest AudioManagerTest#testSoundEffects
Bug: 223815163
Change-Id: Iac6ad878050ffc0cd1faa70abe11d3441ea34d8e
parent cb2985a2
Loading
Loading
Loading
Loading
+206 −58
Original line number Diff line number Diff line
@@ -33,10 +33,156 @@ static struct fields_t {
    jmethodID   mPostEvent;
    jclass      mSoundPoolClass;
} fields;
static inline SoundPool* MusterSoundPool(JNIEnv *env, jobject thiz) {

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<T>
 *           b) TShared, where TShared = std::shared_ptr<T>
 *
 * 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 <typename T>
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<T*>(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)
    return reinterpret_cast<SoundPool*>(env->GetLongField(thiz, fields.mNativeContext));
        auto ptr = reinterpret_cast<T*>(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<SoundPool>
// object with a field in the Java object.
//
// We can then retrieve the std::shared_ptr<SoundPool> 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<std::shared_ptr<SoundPool>> 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>& 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<SoundPool>(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.
}

// ----------------------------------------------------------------------------