BufferQueue: Fix deadlock in setMaxAcquiredBufferCount
Uncovered this while testing. The deadlock happens when:
- ConsumerBase::setMaxAcquiredBufferCount locks itself
- Calls IGBC::setMaxAcquiredBufferCount, which can call
ConsumerListener::onBuffersReleased
- Which, in ConsumerBase, will take the lock again
Instead of this, we add a callback to be called instead of the
IConsumerListener. This callback is called on the same stack, with the
lock held, so that we can resolve everything atomically.
Bug: b/393639203
Flag: EXEMPT small cleanup
Test: new test
Change-Id: Iddd8f902d1fd0aeed6aac095eaa6c0b870ffff70
diff --git a/libs/gui/BufferQueueConsumer.cpp b/libs/gui/BufferQueueConsumer.cpp
index 270bfbd..4681c9e 100644
--- a/libs/gui/BufferQueueConsumer.cpp
+++ b/libs/gui/BufferQueueConsumer.cpp
@@ -14,10 +14,6 @@
* limitations under the License.
*/
-#include <inttypes.h>
-#include <pwd.h>
-#include <sys/types.h>
-
#define LOG_TAG "BufferQueueConsumer"
#define ATRACE_TAG ATRACE_TAG_GRAPHICS
//#define LOG_NDEBUG 0
@@ -48,6 +44,11 @@
#include <com_android_graphics_libgui_flags.h>
+#include <inttypes.h>
+#include <pwd.h>
+#include <sys/types.h>
+#include <optional>
+
namespace android {
// Macros for include BufferQueueCore information in log messages
@@ -767,11 +768,15 @@
return NO_ERROR;
}
+status_t BufferQueueConsumer::setMaxAcquiredBufferCount(int maxAcquiredBuffers) {
+ return setMaxAcquiredBufferCount(maxAcquiredBuffers, std::nullopt);
+}
+
status_t BufferQueueConsumer::setMaxAcquiredBufferCount(
- int maxAcquiredBuffers) {
+ int maxAcquiredBuffers, std::optional<OnBufferReleasedCallback> onBuffersReleasedCallback) {
ATRACE_FORMAT("%s(%d)", __func__, maxAcquiredBuffers);
- sp<IConsumerListener> listener;
+ std::optional<OnBufferReleasedCallback> callback;
{ // Autolock scope
std::unique_lock<std::mutex> lock(mCore->mMutex);
@@ -833,13 +838,20 @@
BQ_LOGV("setMaxAcquiredBufferCount: %d", maxAcquiredBuffers);
mCore->mMaxAcquiredBufferCount = maxAcquiredBuffers;
VALIDATE_CONSISTENCY();
- if (delta < 0 && mCore->mBufferReleasedCbEnabled) {
- listener = mCore->mConsumerListener;
+ if (delta < 0) {
+ if (onBuffersReleasedCallback) {
+ callback = std::move(onBuffersReleasedCallback);
+ } else if (mCore->mBufferReleasedCbEnabled) {
+ callback = [listener = mCore->mConsumerListener]() {
+ listener->onBuffersReleased();
+ };
+ }
}
}
+
// Call back without lock held
- if (listener != nullptr) {
- listener->onBuffersReleased();
+ if (callback) {
+ (*callback)();
}
return NO_ERROR;