BBQ: Clean up acquire states on BQ disconnect
When the producer disconnects, all buffers in the queue will
be freed. So clean up the bbq acquire state and handle any
pending release callbacks. If we do get a release callback for
a pending buffer for a disconnected queue, we cannot
release the buffer back to the queue. So track these
separately and drop the release callbacks as they come.
Transaction callbacks are still expected to come in the
order they were submitted regardless of buffer queue
state. So we can continue to handle the pending
transactions and transaction complete callbacks. When
the queue is reconnected, the queue will increment the
framenumbers starting from the last queued framenumber.
Bug: 201482894
Bug: 197269223
Test: atest BLASTBufferQueueTest
Change-Id: I36791661e2b6112401d85f9b50495931d82125e4
diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp
index dd50ca7..79609fa 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -58,13 +58,22 @@
ALOGE("[%s](f:%u,a:%u) " x, mName.c_str(), mNumFrameAvailable, mNumAcquired, ##__VA_ARGS__)
void BLASTBufferItemConsumer::onDisconnect() {
- Mutex::Autolock lock(mMutex);
- mPreviouslyConnected = mCurrentlyConnected;
- mCurrentlyConnected = false;
- if (mPreviouslyConnected) {
- mDisconnectEvents.push(mCurrentFrameNumber);
+ {
+ Mutex::Autolock lock(mMutex);
+ mPreviouslyConnected = mCurrentlyConnected;
+ mCurrentlyConnected = false;
+ if (mPreviouslyConnected) {
+ mDisconnectEvents.push(mCurrentFrameNumber);
+ }
+ mFrameEventHistory.onDisconnect();
}
- mFrameEventHistory.onDisconnect();
+
+ {
+ std::scoped_lock lock(mBufferQueueMutex);
+ if (mBLASTBufferQueue != nullptr) {
+ mBLASTBufferQueue->onProducerDisconnect();
+ }
+ }
}
void BLASTBufferItemConsumer::addAndGetFrameTimestamps(const NewFrameEventsEntry* newTimestamps,
@@ -202,7 +211,11 @@
}
SurfaceComposerClient::Transaction t;
- const bool setBackpressureFlag = !SurfaceControl::isSameSurface(mSurfaceControl, surface);
+ bool setBackpressureFlag = false;
+ if (!SurfaceControl::isSameSurface(mSurfaceControl, surface)) {
+ mSurfaceControlSwapCount++;
+ setBackpressureFlag = true;
+ }
bool applyTransaction = false;
// Always update the native object even though they might have the same layer handle, so we can
@@ -388,6 +401,19 @@
std::unique_lock _lock{mMutex};
BQA_LOGV("releaseBufferCallback %s", id.to_string().c_str());
+ const auto it = mFreedBuffers.find(id);
+ if (it != mFreedBuffers.end()) {
+ mFreedBuffers.erase(it);
+ BQA_LOGV("releaseBufferCallback ignoring freed buffer %s", id.to_string().c_str());
+ return;
+ }
+
+ if (mFreedBuffers.size() != 0 && mLogMissingReleaseCallback) {
+ BQA_LOGD("Unexpected out of order buffer release. mFreedBuffer count=%d",
+ static_cast<uint32_t>(mFreedBuffers.size()));
+ mLogMissingReleaseCallback = false;
+ }
+
// Calculate how many buffers we need to hold before we release them back
// to the buffer queue. This will prevent higher latency when we are running
// on a lower refresh rate than the max supported. We only do that for EGL
@@ -595,6 +621,12 @@
void BLASTBufferQueue::flushAndWaitForFreeBuffer(std::unique_lock<std::mutex>& lock) {
if (mWaitForTransactionCallback && mNumFrameAvailable > 0) {
+ if ((mSurfaceControlSwapCount > mProducerDisconnectCount) && mLogScSwap) {
+ BQA_LOGD("Expected producer disconnect sc swap count=%d bq disconnect count=%d",
+ mSurfaceControlSwapCount, mProducerDisconnectCount);
+ mLogScSwap = false;
+ }
+
// We are waiting on a previous sync's transaction callback so allow another sync
// transaction to proceed.
//
@@ -1000,4 +1032,31 @@
return mLastAcquiredFrameNumber;
}
+// When the producer disconnects, all buffers in the queue will be freed. So clean up the bbq
+// acquire state and handle any pending release callbacks. If we do get a release callback for a
+// pending buffer for a disconnected queue, we cannot release the buffer back to the queue. So track
+// these separately and drop the release callbacks as they come.
+
+// Transaction callbacks are still expected to come in the order they were submitted regardless of
+// buffer queue state. So we can continue to handles the pending transactions and transaction
+// complete callbacks. When the queue is reconnected, the queue will increment the framenumbers
+// starting from the last queued framenumber.
+void BLASTBufferQueue::onProducerDisconnect() {
+ BQA_LOGV("onProducerDisconnect");
+ std::scoped_lock _lock{mMutex};
+ // reset counts since the queue has been disconnected and all buffers have been freed.
+ mNumFrameAvailable = 0;
+ mNumAcquired = 0;
+
+ // Track submitted buffers in a different container so we can handle any pending release buffer
+ // callbacks without affecting the BBQ acquire state.
+ mFreedBuffers.insert(mSubmitted.begin(), mSubmitted.end());
+ mSubmitted.clear();
+ mPendingRelease.clear();
+ mProducerDisconnectCount++;
+ mCallbackCV.notify_all();
+ mLogMissingReleaseCallback = true;
+ mLogScSwap = true;
+}
+
} // namespace android
diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h
index d766173..82a3483 100644
--- a/libs/gui/include/gui/BLASTBufferQueue.h
+++ b/libs/gui/include/gui/BLASTBufferQueue.h
@@ -109,6 +109,8 @@
uint32_t getLastTransformHint() const;
uint64_t getLastAcquiredFrameNum();
+ void onProducerDisconnect();
+
virtual ~BLASTBufferQueue();
private:
@@ -159,6 +161,12 @@
std::unordered_map<ReleaseCallbackId, BufferItem, ReleaseBufferCallbackIdHash> mSubmitted
GUARDED_BY(mMutex);
+ // Keep a reference to the submitted buffers that were freed so we can drop the buffer quietly
+ // when we get the release callback from flinger. This can happen if the client had disconnected
+ // from the queue.
+ std::unordered_map<ReleaseCallbackId, BufferItem, ReleaseBufferCallbackIdHash> mFreedBuffers
+ GUARDED_BY(mMutex);
+
// Keep a queue of the released buffers instead of immediately releasing
// the buffers back to the buffer queue. This would be controlled by SF
// setting the max acquired buffer count.
@@ -247,6 +255,11 @@
// Flag to determine if syncTransaction should only acquire a single buffer and then clear or
// continue to acquire buffers until explicitly cleared
bool mAcquireSingleBuffer GUARDED_BY(mMutex) = true;
+
+ bool mLogScSwap = true;
+ bool mLogMissingReleaseCallback = true;
+ uint32_t mSurfaceControlSwapCount = 0;
+ uint32_t mProducerDisconnectCount = 0;
};
} // namespace android
diff --git a/services/surfaceflinger/BufferStateLayer.cpp b/services/surfaceflinger/BufferStateLayer.cpp
index 2fac880..046486a 100644
--- a/services/surfaceflinger/BufferStateLayer.cpp
+++ b/services/surfaceflinger/BufferStateLayer.cpp
@@ -404,7 +404,7 @@
nsecs_t desiredPresentTime, bool isAutoTimestamp,
std::optional<nsecs_t> dequeueTime,
const FrameTimelineInfo& info) {
- ATRACE_CALL();
+ ATRACE_NAME(mSetBufferTraceTag.c_str());
const std::shared_ptr<renderengine::ExternalTexture>& buffer =
getBufferFromBufferData(bufferData);
diff --git a/services/surfaceflinger/BufferStateLayer.h b/services/surfaceflinger/BufferStateLayer.h
index eea700c..3aa60826 100644
--- a/services/surfaceflinger/BufferStateLayer.h
+++ b/services/surfaceflinger/BufferStateLayer.h
@@ -153,6 +153,7 @@
static constexpr int kPendingClassificationMaxSurfaceFrames = 25;
const std::string mBlastTransactionName{"BufferTX - " + mName};
+ const std::string mSetBufferTraceTag{"setBuffer - " + mName};
// This integer is incremented everytime a buffer arrives at the server for this layer,
// and decremented when a buffer is dropped or latched. When changed the integer is exported
// to systrace with ATRACE_INT and mBlastTransactionName. This way when debugging perf it is