fix a bug where surfaceflinger and system_server could deadlock
because surfaceflinger handles screenshot in a different
thread from the binder thread that requested it and because
the IGraphicBufferProducer is a synchronous interface
calling back into the system server; it is possible for
the latter to run out of binder threads (b/c it holds
a lock while calling into SF).
The solution is to make sure all calls on IGraphicBufferProducer
happen on the incoming binder thread. We achieve this by creating
a IGBP wrapper which is given to the screenshot code.
Bug: 8734824
Change-Id: I2be85660d9dc65d239d68f6d3ab3c973c13b34cc
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index c6feee1..ecf9fa7 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -2504,11 +2504,92 @@
// Capture screen into an IGraphiBufferProducer
// ---------------------------------------------------------------------------
+/* The code below is here to handle b/8734824
+ *
+ * We create a IGraphicBufferProducer wrapper that forwards all calls
+ * to the calling binder thread, where they are executed. This allows
+ * the calling thread to be reused (on the other side) and not
+ * depend on having "enough" binder threads to handle the requests.
+ *
+ */
+
+class GraphicProducerWrapper : public BBinder, public MessageHandler {
+ sp<IGraphicBufferProducer> impl;
+ sp<Looper> looper;
+ status_t result;
+ bool exitPending;
+ bool exitRequested;
+ mutable Barrier barrier;
+ volatile int32_t memoryBarrier;
+ uint32_t code;
+ Parcel const* data;
+ Parcel* reply;
+
+ enum {
+ MSG_API_CALL,
+ MSG_EXIT
+ };
+
+ /*
+ * this is called by our "fake" BpGraphicBufferProducer. We package the
+ * data and reply Parcel and forward them to the calling thread.
+ */
+ virtual status_t transact(uint32_t code,
+ const Parcel& data, Parcel* reply, uint32_t flags) {
+ this->code = code;
+ this->data = &data;
+ this->reply = reply;
+ android_atomic_acquire_store(0, &memoryBarrier);
+ if (exitPending) {
+ // if we've exited, we run the message synchronously right here
+ handleMessage(Message(MSG_API_CALL));
+ } else {
+ barrier.close();
+ looper->sendMessage(this, Message(MSG_API_CALL));
+ barrier.wait();
+ }
+ return NO_ERROR;
+ }
+
+ /*
+ * here we run on the binder calling thread. All we've got to do is
+ * call the real BpGraphicBufferProducer.
+ */
+ virtual void handleMessage(const Message& message) {
+ android_atomic_release_load(&memoryBarrier);
+ if (message.what == MSG_API_CALL) {
+ impl->asBinder()->transact(code, data[0], reply);
+ barrier.open();
+ } else if (message.what == MSG_EXIT) {
+ exitRequested = true;
+ }
+ }
+
+public:
+ GraphicProducerWrapper(const sp<IGraphicBufferProducer>& impl) :
+ impl(impl), looper(new Looper(true)), result(NO_ERROR),
+ exitPending(false), exitRequested(false) {
+ }
+
+ status_t waitForResponse() {
+ do {
+ looper->pollOnce(-1);
+ } while (!exitRequested);
+ return result;
+ }
+
+ void exit(status_t result) {
+ exitPending = true;
+ looper->sendMessage(this, Message(MSG_EXIT));
+ }
+};
+
+
status_t SurfaceFlinger::captureScreen(const sp<IBinder>& display,
const sp<IGraphicBufferProducer>& producer,
uint32_t reqWidth, uint32_t reqHeight,
uint32_t minLayerZ, uint32_t maxLayerZ,
- bool isCpuConsumer) {
+ bool useReadPixels) {
if (CC_UNLIKELY(display == 0))
return BAD_VALUE;
@@ -2522,18 +2603,18 @@
sp<IGraphicBufferProducer> producer;
uint32_t reqWidth, reqHeight;
uint32_t minLayerZ,maxLayerZ;
- bool isCpuConsumer;
+ bool useReadPixels;
status_t result;
public:
MessageCaptureScreen(SurfaceFlinger* flinger,
const sp<IBinder>& display,
const sp<IGraphicBufferProducer>& producer,
uint32_t reqWidth, uint32_t reqHeight,
- uint32_t minLayerZ, uint32_t maxLayerZ, bool isCpuConsumer)
+ uint32_t minLayerZ, uint32_t maxLayerZ, bool useReadPixels)
: flinger(flinger), display(display), producer(producer),
reqWidth(reqWidth), reqHeight(reqHeight),
minLayerZ(minLayerZ), maxLayerZ(maxLayerZ),
- isCpuConsumer(isCpuConsumer),
+ useReadPixels(useReadPixels),
result(PERMISSION_DENIED)
{
}
@@ -2543,11 +2624,11 @@
virtual bool handler() {
Mutex::Autolock _l(flinger->mStateLock);
sp<const DisplayDevice> hw(flinger->getDisplayDevice(display));
- bool useReadPixels = isCpuConsumer && !flinger->mGpuToCpuSupported;
+ bool useReadPixels = this->useReadPixels && !flinger->mGpuToCpuSupported;
result = flinger->captureScreenImplLocked(hw,
producer, reqWidth, reqHeight, minLayerZ, maxLayerZ,
useReadPixels);
-
+ static_cast<GraphicProducerWrapper*>(producer->asBinder().get())->exit(result);
return true;
}
};
@@ -2559,12 +2640,21 @@
// scheduled at this time, this will end-up being a no-op as well.
mEventQueue.invalidateTransactionNow();
+ // this creates a "fake" BBinder which will serve as a "fake" remote
+ // binder to receive the marshaled calls and forward them to the
+ // real remote (a BpGraphicBufferProducer)
+ sp<GraphicProducerWrapper> wrapper = new GraphicProducerWrapper(producer);
+
+ // the asInterface() call below creates our "fake" BpGraphicBufferProducer
+ // which does the marshaling work forwards to our "fake remote" above.
sp<MessageBase> msg = new MessageCaptureScreen(this,
- display, producer, reqWidth, reqHeight, minLayerZ, maxLayerZ,
- isCpuConsumer);
- status_t res = postMessageSync(msg);
+ display, IGraphicBufferProducer::asInterface( wrapper ),
+ reqWidth, reqHeight, minLayerZ, maxLayerZ,
+ useReadPixels);
+
+ status_t res = postMessageAsync(msg);
if (res == NO_ERROR) {
- res = static_cast<MessageCaptureScreen*>( msg.get() )->getResult();
+ res = wrapper->waitForResponse();
}
return res;
}