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

Commit 1791eefd authored by Dianne Hackborn's avatar Dianne Hackborn Committed by Mathias Agopian
Browse files

fix a couple race-conditions in RefBase::promote()

Bug: 8390295
Change-Id: I7a48e3bf5b213cc1da2b8e844c6bb37ee24cb047
parent 5b00af24
Loading
Loading
Loading
Loading
+67 −26
Original line number Original line Diff line number Diff line
@@ -441,39 +441,68 @@ bool RefBase::weakref_type::attemptIncStrong(const void* id)
    incWeak(id);
    incWeak(id);
    
    
    weakref_impl* const impl = static_cast<weakref_impl*>(this);
    weakref_impl* const impl = static_cast<weakref_impl*>(this);
    
    int32_t curCount = impl->mStrong;
    int32_t curCount = impl->mStrong;
    ALOG_ASSERT(curCount >= 0, "attemptIncStrong called on %p after underflow",

               this);
    ALOG_ASSERT(curCount >= 0,
            "attemptIncStrong called on %p after underflow", this);

    while (curCount > 0 && curCount != INITIAL_STRONG_VALUE) {
    while (curCount > 0 && curCount != INITIAL_STRONG_VALUE) {
        // we're in the easy/common case of promoting a weak-reference
        // from an existing strong reference.
        if (android_atomic_cmpxchg(curCount, curCount+1, &impl->mStrong) == 0) {
        if (android_atomic_cmpxchg(curCount, curCount+1, &impl->mStrong) == 0) {
            break;
            break;
        }
        }
        // the strong count has changed on us, we need to re-assert our
        // situation.
        curCount = impl->mStrong;
        curCount = impl->mStrong;
    }
    }
    
    
    if (curCount <= 0 || curCount == INITIAL_STRONG_VALUE) {
    if (curCount <= 0 || curCount == INITIAL_STRONG_VALUE) {
        bool allow;
        // we're now in the harder case of either:
        if (curCount == INITIAL_STRONG_VALUE) {
        // - there never was a strong reference on us
            // Attempting to acquire first strong reference...  this is allowed
        // - or, all strong references have been released
            // if the object does NOT have a longer lifetime (meaning the
        if ((impl->mFlags&OBJECT_LIFETIME_WEAK) == OBJECT_LIFETIME_STRONG) {
            // implementation doesn't need to see this), or if the implementation
            // this object has a "normal" life-time, i.e.: it gets destroyed
            // allows it to happen.
            // when the last strong reference goes away
            allow = (impl->mFlags&OBJECT_LIFETIME_WEAK) != OBJECT_LIFETIME_WEAK
            if (curCount <= 0) {
                  || impl->mBase->onIncStrongAttempted(FIRST_INC_STRONG, id);
                // the last strong-reference got released, the object cannot
                // be revived.
                decWeak(id);
                return false;
            }

            // here, curCount == INITIAL_STRONG_VALUE, which means
            // there never was a strong-reference, so we can try to
            // promote this object; we need to do that atomically.
            while (curCount > 0) {
                if (android_atomic_cmpxchg(curCount, curCount + 1,
                        &impl->mStrong) == 0) {
                    break;
                }
                // the strong count has changed on us, we need to re-assert our
                // situation (e.g.: another thread has inc/decStrong'ed us)
                curCount = impl->mStrong;
            }

            if (curCount <= 0) {
                // promote() failed, some other thread destroyed us in the
                // meantime (i.e.: strong count reached zero).
                decWeak(id);
                return false;
            }
        } else {
        } else {
            // Attempting to revive the object...  this is allowed
            // this object has an "extended" life-time, i.e.: it can be
            // if the object DOES have a longer lifetime (so we can safely
            // revived from a weak-reference only.
            // call the object with only a weak ref) and the implementation
            // Ask the object's implementation if it agrees to be revived
            // allows it to happen.
            if (!impl->mBase->onIncStrongAttempted(FIRST_INC_STRONG, id)) {
            allow = (impl->mFlags&OBJECT_LIFETIME_WEAK) == OBJECT_LIFETIME_WEAK
                // it didn't so give-up.
                  && impl->mBase->onIncStrongAttempted(FIRST_INC_STRONG, id);
        }
        if (!allow) {
                decWeak(id);
                decWeak(id);
                return false;
                return false;
            }
            }
            // grab a strong-reference, which is always safe due to the
            // extended life-time.
            curCount = android_atomic_inc(&impl->mStrong);
            curCount = android_atomic_inc(&impl->mStrong);
        }


        // If the strong reference count has already been incremented by
        // If the strong reference count has already been incremented by
        // someone else, the implementor of onIncStrongAttempted() is holding
        // someone else, the implementor of onIncStrongAttempted() is holding
@@ -491,9 +520,21 @@ bool RefBase::weakref_type::attemptIncStrong(const void* id)
    ALOGD("attemptIncStrong of %p from %p: cnt=%d\n", this, id, curCount);
    ALOGD("attemptIncStrong of %p from %p: cnt=%d\n", this, id, curCount);
#endif
#endif


    if (curCount == INITIAL_STRONG_VALUE) {
    // now we need to fix-up the count if it was INITIAL_STRONG_VALUE
        android_atomic_add(-INITIAL_STRONG_VALUE, &impl->mStrong);
    // this must be done safely, i.e.: handle the case where several threads
        impl->mBase->onFirstRef();
    // were here in attemptIncStrong().
    curCount = impl->mStrong;
    while (curCount >= INITIAL_STRONG_VALUE) {
        ALOG_ASSERT(curCount > INITIAL_STRONG_VALUE,
                "attemptIncStrong in %p underflowed to INITIAL_STRONG_VALUE",
                this);
        if (android_atomic_cmpxchg(curCount, curCount-INITIAL_STRONG_VALUE,
                &impl->mStrong) == 0) {
            break;
        }
        // the strong-count changed on us, we need to re-assert the situation,
        // for e.g.: it's possible the fix-up happened in another thread.
        curCount = impl->mStrong;
    }
    }


    return true;
    return true;