SF: Trigger ANR when buffer cache is full
* Updates the transaction queue stall listener to take a string that
contains the reason for hanging.
* Updates ClientCache::add to indicate whether or not a failure is due
to the cache being full
* Calls the transaction queue stall listener when the ClientCache is
full
Bug: 244218818
Test: presubmits
Change-Id: I5fdc9aef0f0a1601ace1c42cfac5024c3de8d299
diff --git a/libs/gui/BLASTBufferQueue.cpp b/libs/gui/BLASTBufferQueue.cpp
index 8b9a878..fc8aff8 100644
--- a/libs/gui/BLASTBufferQueue.cpp
+++ b/libs/gui/BLASTBufferQueue.cpp
@@ -167,14 +167,15 @@
mNumFrameAvailable = 0;
TransactionCompletedListener::getInstance()->addQueueStallListener(
- [&]() {
- std::function<void(bool)> callbackCopy;
- {
- std::unique_lock _lock{mMutex};
- callbackCopy = mTransactionHangCallback;
- }
- if (callbackCopy) callbackCopy(true);
- }, this);
+ [&](const std::string& reason) {
+ std::function<void(const std::string&)> callbackCopy;
+ {
+ std::unique_lock _lock{mMutex};
+ callbackCopy = mTransactionHangCallback;
+ }
+ if (callbackCopy) callbackCopy(reason);
+ },
+ this);
BQA_LOGV("BLASTBufferQueue created");
}
@@ -1112,7 +1113,8 @@
return SurfaceControl::isSameSurface(mSurfaceControl, surfaceControl);
}
-void BLASTBufferQueue::setTransactionHangCallback(std::function<void(bool)> callback) {
+void BLASTBufferQueue::setTransactionHangCallback(
+ std::function<void(const std::string&)> callback) {
std::unique_lock _lock{mMutex};
mTransactionHangCallback = callback;
}
diff --git a/libs/gui/ITransactionCompletedListener.cpp b/libs/gui/ITransactionCompletedListener.cpp
index ca91afa..2a114c8 100644
--- a/libs/gui/ITransactionCompletedListener.cpp
+++ b/libs/gui/ITransactionCompletedListener.cpp
@@ -273,15 +273,17 @@
void onReleaseBuffer(ReleaseCallbackId callbackId, sp<Fence> releaseFence,
uint32_t currentMaxAcquiredBufferCount) override {
- callRemoteAsync<decltype(
- &ITransactionCompletedListener::onReleaseBuffer)>(Tag::ON_RELEASE_BUFFER,
- callbackId, releaseFence,
- currentMaxAcquiredBufferCount);
+ callRemoteAsync<decltype(&ITransactionCompletedListener::
+ onReleaseBuffer)>(Tag::ON_RELEASE_BUFFER, callbackId,
+ releaseFence,
+ currentMaxAcquiredBufferCount);
}
- void onTransactionQueueStalled() override {
- callRemoteAsync<decltype(&ITransactionCompletedListener::onTransactionQueueStalled)>(
- Tag::ON_TRANSACTION_QUEUE_STALLED);
+ void onTransactionQueueStalled(const String8& reason) override {
+ callRemoteAsync<
+ decltype(&ITransactionCompletedListener::
+ onTransactionQueueStalled)>(Tag::ON_TRANSACTION_QUEUE_STALLED,
+ reason);
}
};
diff --git a/libs/gui/SurfaceComposerClient.cpp b/libs/gui/SurfaceComposerClient.cpp
index a5879a7..9efdd35 100644
--- a/libs/gui/SurfaceComposerClient.cpp
+++ b/libs/gui/SurfaceComposerClient.cpp
@@ -455,23 +455,24 @@
}
}
-void TransactionCompletedListener::onTransactionQueueStalled() {
- std::unordered_map<void*, std::function<void()>> callbackCopy;
- {
- std::scoped_lock<std::mutex> lock(mMutex);
- callbackCopy = mQueueStallListeners;
- }
- for (auto const& it : callbackCopy) {
- it.second();
- }
+void TransactionCompletedListener::onTransactionQueueStalled(const String8& reason) {
+ std::unordered_map<void*, std::function<void(const std::string&)>> callbackCopy;
+ {
+ std::scoped_lock<std::mutex> lock(mMutex);
+ callbackCopy = mQueueStallListeners;
+ }
+ for (auto const& it : callbackCopy) {
+ it.second(reason.c_str());
+ }
}
-void TransactionCompletedListener::addQueueStallListener(std::function<void()> stallListener,
- void* id) {
+void TransactionCompletedListener::addQueueStallListener(
+ std::function<void(const std::string&)> stallListener, void* id) {
std::scoped_lock<std::mutex> lock(mMutex);
mQueueStallListeners[id] = stallListener;
}
-void TransactionCompletedListener::removeQueueStallListener(void *id) {
+
+void TransactionCompletedListener::removeQueueStallListener(void* id) {
std::scoped_lock<std::mutex> lock(mMutex);
mQueueStallListeners.erase(id);
}
diff --git a/libs/gui/include/gui/BLASTBufferQueue.h b/libs/gui/include/gui/BLASTBufferQueue.h
index 827a6cc..957652e 100644
--- a/libs/gui/include/gui/BLASTBufferQueue.h
+++ b/libs/gui/include/gui/BLASTBufferQueue.h
@@ -113,12 +113,10 @@
uint64_t getLastAcquiredFrameNum();
/**
- * Set a callback to be invoked when we are hung. The boolean parameter
- * indicates whether the hang is due to an unfired fence.
- * TODO: The boolean is always true atm, unfired fence is
- * the only case we detect.
+ * Set a callback to be invoked when we are hung. The string parameter
+ * indicates the reason for the hang.
*/
- void setTransactionHangCallback(std::function<void(bool)> callback);
+ void setTransactionHangCallback(std::function<void(const std::string&)> callback);
virtual ~BLASTBufferQueue();
@@ -282,7 +280,7 @@
bool mAppliedLastTransaction = false;
uint64_t mLastAppliedFrameNumber = 0;
- std::function<void(bool)> mTransactionHangCallback;
+ std::function<void(const std::string&)> mTransactionHangCallback;
std::unordered_set<uint64_t> mSyncedFrameNumbers GUARDED_BY(mMutex);
};
diff --git a/libs/gui/include/gui/ITransactionCompletedListener.h b/libs/gui/include/gui/ITransactionCompletedListener.h
index cc136bb..d2a6f40 100644
--- a/libs/gui/include/gui/ITransactionCompletedListener.h
+++ b/libs/gui/include/gui/ITransactionCompletedListener.h
@@ -194,7 +194,8 @@
virtual void onReleaseBuffer(ReleaseCallbackId callbackId, sp<Fence> releaseFence,
uint32_t currentMaxAcquiredBufferCount) = 0;
- virtual void onTransactionQueueStalled() = 0;
+
+ virtual void onTransactionQueueStalled(const String8& name) = 0;
};
class BnTransactionCompletedListener : public SafeBnInterface<ITransactionCompletedListener> {
diff --git a/libs/gui/include/gui/SurfaceComposerClient.h b/libs/gui/include/gui/SurfaceComposerClient.h
index 36969db..9efde6c 100644
--- a/libs/gui/include/gui/SurfaceComposerClient.h
+++ b/libs/gui/include/gui/SurfaceComposerClient.h
@@ -786,7 +786,7 @@
// This is protected by mSurfaceStatsListenerMutex, but GUARDED_BY isn't supported for
// std::recursive_mutex
std::multimap<int32_t, SurfaceStatsCallbackEntry> mSurfaceStatsListeners;
- std::unordered_map<void*, std::function<void()>> mQueueStallListeners;
+ std::unordered_map<void*, std::function<void(const std::string&)>> mQueueStallListeners;
public:
static sp<TransactionCompletedListener> getInstance();
@@ -804,7 +804,7 @@
const sp<SurfaceControl>& surfaceControl,
const std::unordered_set<CallbackId, CallbackIdHash>& callbackIds);
- void addQueueStallListener(std::function<void()> stallListener, void* id);
+ void addQueueStallListener(std::function<void(const std::string&)> stallListener, void* id);
void removeQueueStallListener(void *id);
/*
@@ -835,7 +835,7 @@
// For Testing Only
static void setInstance(const sp<TransactionCompletedListener>&);
- void onTransactionQueueStalled() override;
+ void onTransactionQueueStalled(const String8& reason) override;
private:
ReleaseBufferCallback popReleaseBufferCallbackLocked(const ReleaseCallbackId&);
diff --git a/services/surfaceflinger/ClientCache.cpp b/services/surfaceflinger/ClientCache.cpp
index b01932e..2bd8f32 100644
--- a/services/surfaceflinger/ClientCache.cpp
+++ b/services/surfaceflinger/ClientCache.cpp
@@ -59,16 +59,17 @@
return true;
}
-bool ClientCache::add(const client_cache_t& cacheId, const sp<GraphicBuffer>& buffer) {
+base::expected<std::shared_ptr<renderengine::ExternalTexture>, ClientCache::AddError>
+ClientCache::add(const client_cache_t& cacheId, const sp<GraphicBuffer>& buffer) {
auto& [processToken, id] = cacheId;
if (processToken == nullptr) {
ALOGE_AND_TRACE("ClientCache::add - invalid (nullptr) process token");
- return false;
+ return base::unexpected(AddError::Unspecified);
}
if (!buffer) {
ALOGE_AND_TRACE("ClientCache::add - invalid (nullptr) buffer");
- return false;
+ return base::unexpected(AddError::Unspecified);
}
std::lock_guard lock(mMutex);
@@ -81,7 +82,7 @@
token = processToken.promote();
if (!token) {
ALOGE_AND_TRACE("ClientCache::add - invalid token");
- return false;
+ return base::unexpected(AddError::Unspecified);
}
// Only call linkToDeath if not a local binder
@@ -89,7 +90,7 @@
status_t err = token->linkToDeath(mDeathRecipient);
if (err != NO_ERROR) {
ALOGE_AND_TRACE("ClientCache::add - could not link to death");
- return false;
+ return base::unexpected(AddError::Unspecified);
}
}
auto [itr, success] =
@@ -104,17 +105,17 @@
if (processBuffers.size() > BUFFER_CACHE_MAX_SIZE) {
ALOGE_AND_TRACE("ClientCache::add - cache is full");
- return false;
+ return base::unexpected(AddError::CacheFull);
}
LOG_ALWAYS_FATAL_IF(mRenderEngine == nullptr,
"Attempted to build the ClientCache before a RenderEngine instance was "
"ready!");
- processBuffers[id].buffer = std::make_shared<
- renderengine::impl::ExternalTexture>(buffer, *mRenderEngine,
- renderengine::impl::ExternalTexture::Usage::
- READABLE);
- return true;
+
+ return (processBuffers[id].buffer = std::make_shared<
+ renderengine::impl::ExternalTexture>(buffer, *mRenderEngine,
+ renderengine::impl::ExternalTexture::
+ Usage::READABLE));
}
void ClientCache::erase(const client_cache_t& cacheId) {
diff --git a/services/surfaceflinger/ClientCache.h b/services/surfaceflinger/ClientCache.h
index a9b8177..cdeac2b 100644
--- a/services/surfaceflinger/ClientCache.h
+++ b/services/surfaceflinger/ClientCache.h
@@ -37,7 +37,10 @@
public:
ClientCache();
- bool add(const client_cache_t& cacheId, const sp<GraphicBuffer>& buffer);
+ enum class AddError { CacheFull, Unspecified };
+
+ base::expected<std::shared_ptr<renderengine::ExternalTexture>, AddError> add(
+ const client_cache_t& cacheId, const sp<GraphicBuffer>& buffer);
void erase(const client_cache_t& cacheId);
std::shared_ptr<renderengine::ExternalTexture> get(const client_cache_t& cacheId);
diff --git a/services/surfaceflinger/FrontEnd/TransactionHandler.cpp b/services/surfaceflinger/FrontEnd/TransactionHandler.cpp
index 6c6a487..95961fe 100644
--- a/services/surfaceflinger/FrontEnd/TransactionHandler.cpp
+++ b/services/surfaceflinger/FrontEnd/TransactionHandler.cpp
@@ -168,15 +168,16 @@
return !mPendingTransactionQueues.empty() || !mLocklessTransactionQueue.isEmpty();
}
-void TransactionHandler::onTransactionQueueStalled(const TransactionState& transaction,
- sp<ITransactionCompletedListener>& listener) {
- if (std::find(mStalledTransactions.begin(), mStalledTransactions.end(), transaction.id) !=
+void TransactionHandler::onTransactionQueueStalled(uint64_t transactionId,
+ sp<ITransactionCompletedListener>& listener,
+ const std::string& reason) {
+ if (std::find(mStalledTransactions.begin(), mStalledTransactions.end(), transactionId) !=
mStalledTransactions.end()) {
return;
}
- mStalledTransactions.push_back(transaction.id);
- listener->onTransactionQueueStalled();
+ mStalledTransactions.push_back(transactionId);
+ listener->onTransactionQueueStalled(String8(reason.c_str()));
}
void TransactionHandler::removeFromStalledTransactions(uint64_t id) {
@@ -185,4 +186,4 @@
mStalledTransactions.erase(it);
}
}
-} // namespace android
\ No newline at end of file
+} // namespace android
diff --git a/services/surfaceflinger/FrontEnd/TransactionHandler.h b/services/surfaceflinger/FrontEnd/TransactionHandler.h
index 237b48d..2b6f07d 100644
--- a/services/surfaceflinger/FrontEnd/TransactionHandler.h
+++ b/services/surfaceflinger/FrontEnd/TransactionHandler.h
@@ -54,7 +54,8 @@
std::vector<TransactionState> flushTransactions();
void addTransactionReadyFilter(TransactionFilter&&);
void queueTransaction(TransactionState&&);
- void onTransactionQueueStalled(const TransactionState&, sp<ITransactionCompletedListener>&);
+ void onTransactionQueueStalled(uint64_t transactionId, sp<ITransactionCompletedListener>&,
+ const std::string& reason);
void removeFromStalledTransactions(uint64_t transactionId);
private:
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 100ad43..d849159 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -3744,7 +3744,10 @@
if (listener &&
(flushState.queueProcessTime - transaction.postTime) >
std::chrono::nanoseconds(4s).count()) {
- mTransactionHandler.onTransactionQueueStalled(transaction, listener);
+ mTransactionHandler
+ .onTransactionQueueStalled(transaction.id, listener,
+ "Buffer processing hung up due to stuck "
+ "fence. Indicates GPU hang");
}
return false;
}
@@ -3960,8 +3963,9 @@
uint32_t clientStateFlags = 0;
for (int i = 0; i < states.size(); i++) {
ComposerState& state = states.editItemAt(i);
- clientStateFlags |= setClientStateLocked(frameTimelineInfo, state, desiredPresentTime,
- isAutoTimestamp, postTime, permissions);
+ clientStateFlags |=
+ setClientStateLocked(frameTimelineInfo, state, desiredPresentTime, isAutoTimestamp,
+ postTime, permissions, transactionId);
if ((flags & eAnimation) && state.state.surface) {
if (const auto layer = fromHandle(state.state.surface).promote()) {
using LayerUpdateType = scheduler::LayerHistory::LayerUpdateType;
@@ -4077,7 +4081,8 @@
uint32_t SurfaceFlinger::setClientStateLocked(const FrameTimelineInfo& frameTimelineInfo,
ComposerState& composerState,
int64_t desiredPresentTime, bool isAutoTimestamp,
- int64_t postTime, uint32_t permissions) {
+ int64_t postTime, uint32_t permissions,
+ uint64_t transactionId) {
layer_state_t& s = composerState.state;
s.sanitize(permissions);
@@ -4370,7 +4375,8 @@
if (what & layer_state_t::eBufferChanged) {
std::shared_ptr<renderengine::ExternalTexture> buffer =
- getExternalTextureFromBufferData(*s.bufferData, layer->getDebugName());
+ getExternalTextureFromBufferData(*s.bufferData, layer->getDebugName(),
+ transactionId);
if (layer->setBuffer(buffer, *s.bufferData, postTime, desiredPresentTime, isAutoTimestamp,
dequeueBufferTimestamp, frameTimelineInfo)) {
flags |= eTraversalNeeded;
@@ -6955,34 +6961,44 @@
}
std::shared_ptr<renderengine::ExternalTexture> SurfaceFlinger::getExternalTextureFromBufferData(
- const BufferData& bufferData, const char* layerName) const {
- bool cacheIdChanged = bufferData.flags.test(BufferData::BufferDataChange::cachedBufferChanged);
- bool bufferSizeExceedsLimit = false;
- std::shared_ptr<renderengine::ExternalTexture> buffer = nullptr;
- if (cacheIdChanged && bufferData.buffer != nullptr) {
- bufferSizeExceedsLimit = exceedsMaxRenderTargetSize(bufferData.buffer->getWidth(),
- bufferData.buffer->getHeight());
- if (!bufferSizeExceedsLimit) {
- ClientCache::getInstance().add(bufferData.cachedBuffer, bufferData.buffer);
- buffer = ClientCache::getInstance().get(bufferData.cachedBuffer);
- }
- } else if (cacheIdChanged) {
- buffer = ClientCache::getInstance().get(bufferData.cachedBuffer);
- } else if (bufferData.buffer != nullptr) {
- bufferSizeExceedsLimit = exceedsMaxRenderTargetSize(bufferData.buffer->getWidth(),
- bufferData.buffer->getHeight());
- if (!bufferSizeExceedsLimit) {
- buffer = std::make_shared<
- renderengine::impl::ExternalTexture>(bufferData.buffer, getRenderEngine(),
- renderengine::impl::ExternalTexture::
- Usage::READABLE);
- }
+ BufferData& bufferData, const char* layerName, uint64_t transactionId) {
+ if (bufferData.buffer &&
+ exceedsMaxRenderTargetSize(bufferData.buffer->getWidth(), bufferData.buffer->getHeight())) {
+ ALOGE("Attempted to create an ExternalTexture for layer %s that exceeds render target "
+ "size limit.",
+ layerName);
+ return nullptr;
}
- ALOGE_IF(bufferSizeExceedsLimit,
- "Attempted to create an ExternalTexture for layer %s that exceeds render target size "
- "limit.",
- layerName);
- return buffer;
+
+ bool cachedBufferChanged =
+ bufferData.flags.test(BufferData::BufferDataChange::cachedBufferChanged);
+ if (cachedBufferChanged && bufferData.buffer) {
+ auto result = ClientCache::getInstance().add(bufferData.cachedBuffer, bufferData.buffer);
+ if (result.ok()) {
+ return result.value();
+ }
+
+ if (result.error() == ClientCache::AddError::CacheFull) {
+ mTransactionHandler
+ .onTransactionQueueStalled(transactionId, bufferData.releaseBufferListener,
+ "Buffer processing hung due to full buffer cache");
+ }
+
+ return nullptr;
+ }
+
+ if (cachedBufferChanged) {
+ return ClientCache::getInstance().get(bufferData.cachedBuffer);
+ }
+
+ if (bufferData.buffer) {
+ return std::make_shared<
+ renderengine::impl::ExternalTexture>(bufferData.buffer, getRenderEngine(),
+ renderengine::impl::ExternalTexture::Usage::
+ READABLE);
+ }
+
+ return nullptr;
}
bool SurfaceFlinger::commitMirrorDisplays(VsyncId vsyncId) {
diff --git a/services/surfaceflinger/SurfaceFlinger.h b/services/surfaceflinger/SurfaceFlinger.h
index 674f21f..3a14ba0 100644
--- a/services/surfaceflinger/SurfaceFlinger.h
+++ b/services/surfaceflinger/SurfaceFlinger.h
@@ -312,7 +312,7 @@
REQUIRES(mStateLock);
virtual std::shared_ptr<renderengine::ExternalTexture> getExternalTextureFromBufferData(
- const BufferData& bufferData, const char* layerName) const;
+ BufferData& bufferData, const char* layerName, uint64_t transactionId);
// Returns true if any display matches a `bool(const DisplayDevice&)` predicate.
template <typename Predicate>
@@ -728,7 +728,8 @@
uint32_t setClientStateLocked(const FrameTimelineInfo&, ComposerState&,
int64_t desiredPresentTime, bool isAutoTimestamp,
- int64_t postTime, uint32_t permissions) REQUIRES(mStateLock);
+ int64_t postTime, uint32_t permissions, uint64_t transactionId)
+ REQUIRES(mStateLock);
uint32_t getTransactionFlags() const;
diff --git a/services/surfaceflinger/Tracing/tools/LayerTraceGenerator.cpp b/services/surfaceflinger/Tracing/tools/LayerTraceGenerator.cpp
index 276b3da..3b11f23 100644
--- a/services/surfaceflinger/Tracing/tools/LayerTraceGenerator.cpp
+++ b/services/surfaceflinger/Tracing/tools/LayerTraceGenerator.cpp
@@ -100,7 +100,8 @@
MockSurfaceFlinger(Factory& factory)
: SurfaceFlinger(factory, SurfaceFlinger::SkipInitialization) {}
std::shared_ptr<renderengine::ExternalTexture> getExternalTextureFromBufferData(
- const BufferData& bufferData, const char* /* layerName */) const override {
+ BufferData& bufferData, const char* /* layerName */,
+ uint64_t /* transactionId */) override {
return std::make_shared<renderengine::mock::FakeExternalTexture>(bufferData.getWidth(),
bufferData.getHeight(),
bufferData.getId(),