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

Commit 30da367c authored by Patrick Williams's avatar Patrick Williams
Browse files

Ensure transaction commit callbacks are called at most once. 

Bug: 288781573
Bug: 292443283
Test: atest LayerTransactionTest
Change-Id: Ide66601b8a892b4bf5a331d83d38b4bd50919751
parent d3208d8f

libs/gui/SurfaceComposerClient.cpp

+5 −1
Original line number Diff line number Diff line
@@ -377,7 +377,6 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener 
            }

            auto& [callbackFunction, callbackSurfaceControls] = callbacksMap[callbackId];

            if (!callbackFunction) {

                ALOGE("cannot call null callback function, skipping");

                continue;

            }

            std::vector<SurfaceControlStats> surfaceControlStats;
@@ -394,6 +393,11 @@ void TransactionCompletedListener::onTransactionCompleted(ListenerStats listener 


            callbackFunction(transactionStats.latchTime, transactionStats.presentFence,

                             surfaceControlStats);



            // More than one transaction may contain the same callback id. Erase the callback from

            // the map to ensure that it is only called once. This can happen if transactions are

            // parcelled out of process and applied in both processes.

            callbacksMap.erase(callbackId);

        }



        // handle on complete callbacks

services/surfaceflinger/tests/LayerTransaction_test.cpp

+29 −0
Original line number Diff line number Diff line
@@ -184,6 +184,35 @@ TEST_F(LayerTransactionTest, BufferTakesPriorityOverColor) { 
    }

}



TEST_F(LayerTransactionTest, CommitCallbackCalledOnce) {

    auto callCount = 0;

    auto commitCallback =

            [&callCount](void* /* context */, nsecs_t /* latchTime */,

                         const sp<Fence>& /* presentFence */,

                         const std::vector<SurfaceControlStats>& /* stats */) mutable {

                callCount++;

            };



    // Create two transactions that both contain the same callback id.

    Transaction t1;

    t1.addTransactionCommittedCallback(commitCallback, nullptr);

    Parcel parcel;

    t1.writeToParcel(&parcel);

    parcel.setDataPosition(0);

    Transaction t2;

    t2.readFromParcel(&parcel);



    // Apply the two transactions. There is a race here as we can't guarantee that the two

    // transactions will be applied within the same SurfaceFlinger commit. If the transactions are

    // applied within the same commit then we verify that callback ids are deduplicated within a

    // single commit. Otherwise, we verify that commit callbacks are deduplicated across separate

    // commits.

    t1.apply();

    t2.apply(/*synchronous=*/true);



    ASSERT_EQ(callCount, 1);

}



} // namespace android



// TODO(b/129481165): remove the #pragma below and fix conversion issues