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

Commit e7b86a1e authored by Steven Moreland's avatar Steven Moreland Committed by Android (Google) Code Review
Browse files

Merge changes I11873247,Iaa8367ed,I126ee20d into main

* changes:
  BinderProxy: 'object' vs 'recipient'
  Binder death recipients: clarify comments
  log for double-creation of BinderProxyNativeData.
parents 145fbe21 d4611b21
Loading
Loading
Loading
Loading
+44 −45
Original line number Diff line number Diff line
@@ -631,13 +631,13 @@ constexpr const char* logPrefix<IBinder::FrozenStateChangeCallback>() {
template <typename T>
class JavaRecipient : public T {
public:
    JavaRecipient(JNIEnv* env, jobject object, const sp<RecipientList<T> >& list,
    JavaRecipient(JNIEnv* env, jobject recipient, const sp<RecipientList<T>>& list,
                  bool useWeakReference)
          : mVM(jnienv_to_javavm(env)), mObject(NULL), mObjectWeak(NULL), mList(list) {
          : mVM(jnienv_to_javavm(env)), mRecipient(NULL), mRecipientWeak(NULL), mList(list) {
        if (useWeakReference) {
            mObjectWeak = env->NewWeakGlobalRef(object);
            mRecipientWeak = env->NewWeakGlobalRef(recipient);
        } else {
            mObject = env->NewGlobalRef(object);
            mRecipient = env->NewGlobalRef(recipient);
        }
    }

@@ -668,21 +668,19 @@ public:
        bool result;
        JNIEnv* env = javavm_to_jnienv(mVM);

        if (mObject != NULL) {
            result = env->IsSameObject(obj, mObject);
        if (mRecipient != NULL) {
            result = env->IsSameObject(obj, mRecipient);
        } else {
            ScopedLocalRef<jobject> me(env, env->NewLocalRef(mObjectWeak));
            ScopedLocalRef<jobject> me(env, env->NewLocalRef(mRecipientWeak));
            result = env->IsSameObject(obj, me.get());
        }
        return result;
    }

    void warnIfStillLive() {
        if (mObject != NULL) {
            // Okay, something is wrong -- we have a hard reference to a live death
            // recipient on the VM side, but the list is being torn down.
        if (mRecipient != NULL) {
            JNIEnv* env = javavm_to_jnienv(mVM);
            ScopedLocalRef<jclass> objClassRef(env, env->GetObjectClass(mObject));
            ScopedLocalRef<jclass> objClassRef(env, env->GetObjectClass(mRecipient));
            ScopedLocalRef<jstring> nameRef(env,
                                            (jstring)env->CallObjectMethod(objClassRef.get(),
                                                                           gClassOffsets.mGetName));
@@ -701,27 +699,26 @@ public:

protected:
    virtual ~JavaRecipient() {
        // ALOGI("Removing death ref: recipient=%p\n", mObject);
        // ALOGI("Removing death ref: recipient=%p\n", mRecipient);
        JNIEnv* env = javavm_to_jnienv(mVM);
        if (mObject != NULL) {
            env->DeleteGlobalRef(mObject);
        if (mRecipient != NULL) {
            env->DeleteGlobalRef(mRecipient);
        } else {
            env->DeleteWeakGlobalRef(mObjectWeak);
            env->DeleteWeakGlobalRef(mRecipientWeak);
        }
    }

    JavaVM* const mVM;

    // If useWeakReference is false (e.g. JavaDeathRecipient when target sdk version < 35), the
    // Java-side Recipient is strongly referenced from mObject initially, and may later be demoted
    // to a weak reference (mObjectWeak), e.g. upon linkToDeath() and then after binderDied() is
    // called.
    // If useWeakReference is true, the strong reference is never made here (i.e. mObject == NULL
    // always). Instead, the strong reference to the Java-side Recipient is made in
    // BinderProxy.{mDeathRecipients,mFrozenStateChangeCallbacks}. In the native world, only the
    // weak reference is kept.
    jobject mObject;
    jweak mObjectWeak;
    // Java-side Recipient is strongly referenced from mRecipient initially, and may later be
    // demoted to a weak reference (mRecipientWeak), e.g. upon linkToDeath() and then after
    // binderDied() is called. If useWeakReference is true, the strong reference is never made here
    // (i.e. mRecipient == NULL always). Instead, the strong reference to the Java-side Recipient is
    // made in BinderProxy.{mDeathRecipients,mFrozenStateChangeCallbacks}. In the native world, only
    // the weak reference is kept.
    jobject mRecipient;
    jweak mRecipientWeak;
    wp<RecipientList<T> > mList;
};

@@ -749,9 +746,9 @@ public:
#endif
    }

    JavaDeathRecipient(JNIEnv* env, jobject object,
    JavaDeathRecipient(JNIEnv* env, jobject recipient,
                       const sp<RecipientList<IBinder::DeathRecipient>>& list)
          : JavaRecipient(env, object, list, useWeakReference()) {
          : JavaRecipient(env, recipient, list, useWeakReference()) {
        gNumDeathRefsCreated.fetch_add(1, std::memory_order_relaxed);
        gcIfManyNewRefs(env);
    }
@@ -763,7 +760,7 @@ public:
    void binderDied(const wp<IBinder>& who)
    {
        LOG_DEATH_FREEZE("Receiving binderDied() on JavaDeathRecipient %p\n", this);
        if (mObject == NULL && mObjectWeak == NULL) {
        if (mRecipient == NULL && mRecipientWeak == NULL) {
            return;
        }
        JNIEnv* env = javavm_to_jnienv(mVM);
@@ -772,8 +769,8 @@ public:
        // Hold a local reference to the recipient. This may fail if the recipient is weakly
        // referenced, in which case we can't deliver the death notice.
        ScopedLocalRef<jobject> jRecipient(env,
                                           env->NewLocalRef(mObject != NULL ? mObject
                                                                            : mObjectWeak));
                                           env->NewLocalRef(mRecipient != NULL ? mRecipient
                                                                               : mRecipientWeak));
        if (jRecipient.get() == NULL) {
            ALOGW("Binder died, but death recipient is already garbage collected. If your target "
                  "sdk level is at or above 35, this can happen when you dropped all references to "
@@ -799,16 +796,16 @@ public:

        // Demote from strong ref (if exists) to weak after binderDied() has been delivered, to
        // allow the DeathRecipient and BinderProxy to be GC'd if no longer needed. Do this in sync
        // with our containing DeathRecipientList so that we can't delete the global ref on mObject
        // while the list is being iterated.
        if (mObject != NULL) {
        // with our containing DeathRecipientList so that we can't delete the global ref on
        // mRecipient while the list is being iterated.
        if (mRecipient != NULL) {
            auto list = mList.promote();
            if (list != NULL) {
                AutoMutex _l(list->lock());

                mObjectWeak = env->NewWeakGlobalRef(mObject);
                env->DeleteGlobalRef(mObject);
                mObject = NULL;
                mRecipientWeak = env->NewWeakGlobalRef(mRecipient);
                env->DeleteGlobalRef(mRecipient);
                mRecipient = NULL;
            }
        }
    }
@@ -820,16 +817,15 @@ private:

class JavaFrozenStateChangeCallback : public JavaRecipient<IBinder::FrozenStateChangeCallback> {
public:
    JavaFrozenStateChangeCallback(
            JNIEnv* env, jobject object,
    JavaFrozenStateChangeCallback(JNIEnv* env, jobject recipient /*a.k.a callback*/,
                                  const sp<RecipientList<IBinder::FrozenStateChangeCallback>>& list)
          : JavaRecipient(env, object, list, /*useWeakReference=*/true) {}
          : JavaRecipient(env, recipient, list, /*useWeakReference=*/true) {}

    void onStateChanged(const wp<IBinder>& who, State state) {
        LOG_DEATH_FREEZE("Receiving onStateChanged() on JavaFrozenStateChangeCallback %p. state: "
                         "%s\n",
                         this, state == State::FROZEN ? "FROZEN" : "UNFROZEN");
        if (mObjectWeak == NULL) {
        if (mRecipientWeak == NULL) {
            return;
        }
        JNIEnv* env = javavm_to_jnienv(mVM);
@@ -837,7 +833,7 @@ public:

        // Hold a local reference to the recipient. This may fail if the recipient is weakly
        // referenced, in which case we can't deliver the notification.
        ScopedLocalRef<jobject> jCallback(env, env->NewLocalRef(mObjectWeak));
        ScopedLocalRef<jobject> jCallback(env, env->NewLocalRef(mRecipientWeak));
        if (jCallback.get() == NULL) {
            return;
        }
@@ -865,9 +861,9 @@ RecipientList<T>::~RecipientList() {
    LOG_DEATH_FREEZE("%s Destroy RecipientList @ %p", logPrefix<T>(), this);
    AutoMutex _l(mLock);

    // Should never happen -- the JavaRecipientList objects that have added themselves
    // to the list are holding references on the list object.  Only when they are torn
    // down can the list header be destroyed.
    // RecipientList recipients hold a weak reference to this object. If
    // this list is destroyed first, it means that unlinkToDeath is not
    // called. Warn them.
    if (mList.size() > 0) {
        for (auto iter = mList.begin(); iter != mList.end(); iter++) {
            (*iter)->warnIfStillLive();
@@ -977,6 +973,9 @@ jobject javaObjectForIBinder(JNIEnv* env, const sp<IBinder>& val)
    BinderProxyNativeData* actualNativeData = getBPNativeData(env, object);
    if (actualNativeData != nativeData) {
        delete nativeData;

        // added to help debug bugs like b/405737267, feel free to delete
        ALOGE("Two threads raced to create BinderProxyNativeData.");
    }

    return object;