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

Commit 0442a869 authored by Martijn Coenen's avatar Martijn Coenen Committed by Steven Moreland
Browse files

Flush BC_FREE_BUFFER and ref ops from non-looper threads. 

BC_FREE_BUFFER and ref commands are normally just queued, and not
automatically flushed out to the kernel driver. This usually works fine,
because BC_FREE_BUFFER is typically called from a binder thread (which
flushes when calling back into the kernel), or a thread making regular
binder transactions itself.

But it can happen that a Parcel is destructed from a thread that meets
neither of those requirements; especially Parcels created from Java are
sensitive to this, because if they aren't immediately recycled, they
will instead be garbage collected, and in that case the BC_FREE_BUFFER
will be queued to the FinalizerDaemon thread, which otherwise never
makes or receives any binder calls.

To prevent these commands from getting stuck, flush BC_FREE_BUFFER and
refcount operations automatically from such threads.

Bug: 68604253
Bug: 139697085
Test: boots, binderLibTest
Change-Id: I98109a7046c122db22af0b15a268629284f06663
parent 16b0b20f

libs/binder/IPCThreadState.cpp

+29 −6
Original line number Diff line number Diff line
@@ -486,6 +486,19 @@ void IPCThreadState::flushCommands() 
    }

}



bool IPCThreadState::flushIfNeeded()

{

    if (mIsLooper || mServingStackPointer != nullptr) {

        return false;

    }

    // In case this thread is not a looper and is not currently serving a binder transaction,

    // there's no guarantee that this thread will call back into the kernel driver any time

    // soon. Therefore, flush pending commands such as BC_FREE_BUFFER, to prevent them from getting

    // stuck in this thread's out buffer.

    flushCommands();

    return true;

}



void IPCThreadState::blockUntilThreadAvailable()

{

    pthread_mutex_lock(&mProcess->mThreadCountLock);
@@ -597,6 +610,7 @@ void IPCThreadState::joinThreadPool(bool isMain) 


    mOut.writeInt32(isMain ? BC_ENTER_LOOPER : BC_REGISTER_LOOPER);



    mIsLooper = true;

    status_t result;

    do {

        processPendingDerefs();
@@ -619,6 +633,7 @@ void IPCThreadState::joinThreadPool(bool isMain) 
        (void*)pthread_self(), getpid(), result);



    mOut.writeInt32(BC_EXIT_LOOPER);

    mIsLooper = false;

    talkWithDriver(false);

}
@@ -731,16 +746,19 @@ void IPCThreadState::incStrongHandle(int32_t handle, BpBinder *proxy) 
    LOG_REMOTEREFS("IPCThreadState::incStrongHandle(%d)\n", handle);

    mOut.writeInt32(BC_ACQUIRE);

    mOut.writeInt32(handle);

    if (!flushIfNeeded()) {

        // Create a temp reference until the driver has handled this command.

        proxy->incStrong(mProcess.get());

        mPostWriteStrongDerefs.push(proxy);

    }

}



void IPCThreadState::decStrongHandle(int32_t handle)

{

    LOG_REMOTEREFS("IPCThreadState::decStrongHandle(%d)\n", handle);

    mOut.writeInt32(BC_RELEASE);

    mOut.writeInt32(handle);

    flushIfNeeded();

}



void IPCThreadState::incWeakHandle(int32_t handle, BpBinder *proxy)
@@ -748,16 +766,19 @@ void IPCThreadState::incWeakHandle(int32_t handle, BpBinder *proxy) 
    LOG_REMOTEREFS("IPCThreadState::incWeakHandle(%d)\n", handle);

    mOut.writeInt32(BC_INCREFS);

    mOut.writeInt32(handle);

    if (!flushIfNeeded()) {

        // Create a temp reference until the driver has handled this command.

        proxy->getWeakRefs()->incWeak(mProcess.get());

        mPostWriteWeakDerefs.push(proxy->getWeakRefs());

    }

}



void IPCThreadState::decWeakHandle(int32_t handle)

{

    LOG_REMOTEREFS("IPCThreadState::decWeakHandle(%d)\n", handle);

    mOut.writeInt32(BC_DECREFS);

    mOut.writeInt32(handle);

    flushIfNeeded();

}



status_t IPCThreadState::attemptIncStrongHandle(int32_t handle)
@@ -813,6 +834,7 @@ IPCThreadState::IPCThreadState() 
      mServingStackPointer(nullptr),

      mWorkSource(kUnsetWorkSource),

      mPropagateWorkSource(false),

      mIsLooper(false),

      mStrictModePolicy(0),

      mLastTransactionBinderFlags(0),

      mCallRestriction(mProcess->mCallRestriction)
@@ -1393,6 +1415,7 @@ void IPCThreadState::freeBuffer(Parcel* parcel, const uint8_t* data, 
    IPCThreadState* state = self();

    state->mOut.writeInt32(BC_FREE_BUFFER);

    state->mOut.writePointer((uintptr_t)data);

    state->flushIfNeeded();

}



} // namespace android

libs/binder/include/binder/IPCThreadState.h

+2 −0
Original line number Diff line number Diff line
@@ -110,6 +110,7 @@ public: 
            status_t            setupPolling(int* fd);

            status_t            handlePolledCommands();

            void                flushCommands();

            bool                flushIfNeeded();



            void                joinThreadPool(bool isMain = true);
@@ -204,6 +205,7 @@ private: 
            int32_t             mWorkSource;

            // Whether the work source should be propagated.

            bool                mPropagateWorkSource;

            bool                mIsLooper;

            int32_t             mStrictModePolicy;

            int32_t             mLastTransactionBinderFlags;

            CallRestriction     mCallRestriction;

libs/binder/ndk/tests/libbinder_ndk_unit_test.cpp

+1 −2
Original line number Diff line number Diff line
@@ -375,8 +375,7 @@ TEST(NdkBinder, ABpBinderRefCount) { 


    AIBinder_decStrong(binder);



    // assert because would need to decStrong if non-null and we shouldn't need to add a no-op here

    ASSERT_NE(nullptr, AIBinder_Weak_promote(wBinder));

    ASSERT_EQ(nullptr, AIBinder_Weak_promote(wBinder));



    AIBinder_Weak_delete(wBinder);

}