Merge "Add warning logs when we fail to set BufferReleaseChannel" into main

Author: patrickdwilliams

libs/gui/BLASTBufferQueue.cpp

@@ -294,12 +294,7 @@ void BLASTBufferQueue::update(const sp<SurfaceControl>& surface, uint32_t width,
     SurfaceComposerClient::Transaction t;
     if (surfaceControlChanged) {
 #if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
-        // SELinux policy may prevent this process from sending the BufferReleaseChannel's file
-        // descriptor to SurfaceFlinger, causing the entire transaction to be dropped. This
-        // transaction is applied separately to ensure we don't lose the other updates.
-        t.setApplyToken(mApplyToken)
-                .setBufferReleaseChannel(mSurfaceControl, mBufferReleaseProducer)
-                .apply(false /* synchronous */, true /* oneWay */);
+        updateBufferReleaseProducer();
 #endif
         t.setFlags(mSurfaceControl, layer_state_t::eEnableBackpressure,
                    layer_state_t::eEnableBackpressure);
@@ -1335,6 +1330,20 @@ void BLASTBufferQueue::setApplyToken(sp<IBinder> applyToken) {
 
 #if COM_ANDROID_GRAPHICS_LIBGUI_FLAGS(BUFFER_RELEASE_CHANNEL)
 
+void BLASTBufferQueue::updateBufferReleaseProducer() {
+    // SELinux policy may prevent this process from sending the BufferReleaseChannel's file
+    // descriptor to SurfaceFlinger, causing the entire transaction to be dropped. We send this
+    // transaction independently of any other updates to ensure those updates aren't lost.
+    SurfaceComposerClient::Transaction t;
+    status_t status = t.setApplyToken(mApplyToken)
+                              .setBufferReleaseChannel(mSurfaceControl, mBufferReleaseProducer)
+                              .apply(false /* synchronous */, true /* oneWay */);
+    if (status != OK) {
+        ALOGW("[%s] %s - failed to set buffer release channel on %s", mName.c_str(),
+              statusToString(status).c_str(), mSurfaceControl->getName().c_str());
+    }
+}
+
 void BLASTBufferQueue::drainBufferReleaseConsumer() {
     ATRACE_CALL();
     while (true) {

libs/gui/SurfaceComposerClient.cpp

@@ -1347,21 +1347,22 @@ status_t SurfaceComposerClient::Transaction::apply(bool synchronous, bool oneWay
     sp<IBinder> applyToken = mApplyToken ? mApplyToken : getDefaultApplyToken();
 
     sp<ISurfaceComposer> sf(ComposerService::getComposerService());
-    sf->setTransactionState(mFrameTimelineInfo, composerStates, displayStates, flags, applyToken,
-                            mInputWindowCommands, mDesiredPresentTime, mIsAutoTimestamp,
-                            mUncacheBuffers, hasListenerCallbacks, listenerCallbacks, mId,
-                            mMergedTransactionIds);
+    status_t binderStatus =
+            sf->setTransactionState(mFrameTimelineInfo, composerStates, displayStates, flags,
+                                    applyToken, mInputWindowCommands, mDesiredPresentTime,
+                                    mIsAutoTimestamp, mUncacheBuffers, hasListenerCallbacks,
+                                    listenerCallbacks, mId, mMergedTransactionIds);
     mId = generateId();
 
     // Clear the current states and flags
     clear();
 
-    if (synchronous) {
+    if (synchronous && binderStatus == OK) {
         syncCallback->wait();
     }
 
     mStatus = NO_ERROR;
-    return NO_ERROR;
+    return binderStatus;
 }
 
 sp<IBinder> SurfaceComposerClient::Transaction::sApplyToken = new BBinder();
@@ -1375,7 +1376,7 @@ sp<IBinder> SurfaceComposerClient::Transaction::getDefaultApplyToken() {
 
 void SurfaceComposerClient::Transaction::setDefaultApplyToken(sp<IBinder> applyToken) {
     std::scoped_lock lock{sApplyTokenMutex};
-    sApplyToken = applyToken;
+    sApplyToken = std::move(applyToken);
 }
 
 status_t SurfaceComposerClient::Transaction::sendSurfaceFlushJankDataTransaction(

libs/gui/include/gui/BLASTBufferQueue.h

@@ -325,8 +325,14 @@ class BLASTBufferQueue : public ConsumerBase::FrameAvailableListener {
     std::unique_ptr<gui::BufferReleaseChannel::ConsumerEndpoint> mBufferReleaseConsumer;
     std::shared_ptr<gui::BufferReleaseChannel::ProducerEndpoint> mBufferReleaseProducer;
 
+    void updateBufferReleaseProducer() REQUIRES(mMutex);
     void drainBufferReleaseConsumer();
 
+    // BufferReleaseReader is used to do blocking but interruptible reads from the buffer
+    // release channel. To implement this, BufferReleaseReader owns an epoll file descriptor that
+    // is configured to wake up when either the BufferReleaseReader::ConsumerEndpoint or an eventfd
+    // becomes readable. Interrupts are necessary because a free buffer may become available for
+    // reasons other than a buffer release from the producer.
     class BufferReleaseReader {
     public:
         explicit BufferReleaseReader(BLASTBufferQueue&);