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
This commit is contained in:
Mathias Agopian 2013-07-08 17:09:41 -07:00
parent 547e98f33c
commit 2ed0fe59b4

View File

@ -2504,11 +2504,92 @@ void SurfaceFlinger::repaintEverything() {
// Capture screen into an IGraphiBufferProducer // 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, status_t SurfaceFlinger::captureScreen(const sp<IBinder>& display,
const sp<IGraphicBufferProducer>& producer, const sp<IGraphicBufferProducer>& producer,
uint32_t reqWidth, uint32_t reqHeight, uint32_t reqWidth, uint32_t reqHeight,
uint32_t minLayerZ, uint32_t maxLayerZ, uint32_t minLayerZ, uint32_t maxLayerZ,
bool isCpuConsumer) { bool useReadPixels) {
if (CC_UNLIKELY(display == 0)) if (CC_UNLIKELY(display == 0))
return BAD_VALUE; return BAD_VALUE;
@ -2522,18 +2603,18 @@ status_t SurfaceFlinger::captureScreen(const sp<IBinder>& display,
sp<IGraphicBufferProducer> producer; sp<IGraphicBufferProducer> producer;
uint32_t reqWidth, reqHeight; uint32_t reqWidth, reqHeight;
uint32_t minLayerZ,maxLayerZ; uint32_t minLayerZ,maxLayerZ;
bool isCpuConsumer; bool useReadPixels;
status_t result; status_t result;
public: public:
MessageCaptureScreen(SurfaceFlinger* flinger, MessageCaptureScreen(SurfaceFlinger* flinger,
const sp<IBinder>& display, const sp<IBinder>& display,
const sp<IGraphicBufferProducer>& producer, const sp<IGraphicBufferProducer>& producer,
uint32_t reqWidth, uint32_t reqHeight, 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), : flinger(flinger), display(display), producer(producer),
reqWidth(reqWidth), reqHeight(reqHeight), reqWidth(reqWidth), reqHeight(reqHeight),
minLayerZ(minLayerZ), maxLayerZ(maxLayerZ), minLayerZ(minLayerZ), maxLayerZ(maxLayerZ),
isCpuConsumer(isCpuConsumer), useReadPixels(useReadPixels),
result(PERMISSION_DENIED) result(PERMISSION_DENIED)
{ {
} }
@ -2543,11 +2624,11 @@ status_t SurfaceFlinger::captureScreen(const sp<IBinder>& display,
virtual bool handler() { virtual bool handler() {
Mutex::Autolock _l(flinger->mStateLock); Mutex::Autolock _l(flinger->mStateLock);
sp<const DisplayDevice> hw(flinger->getDisplayDevice(display)); sp<const DisplayDevice> hw(flinger->getDisplayDevice(display));
bool useReadPixels = isCpuConsumer && !flinger->mGpuToCpuSupported; bool useReadPixels = this->useReadPixels && !flinger->mGpuToCpuSupported;
result = flinger->captureScreenImplLocked(hw, result = flinger->captureScreenImplLocked(hw,
producer, reqWidth, reqHeight, minLayerZ, maxLayerZ, producer, reqWidth, reqHeight, minLayerZ, maxLayerZ,
useReadPixels); useReadPixels);
static_cast<GraphicProducerWrapper*>(producer->asBinder().get())->exit(result);
return true; return true;
} }
}; };
@ -2559,12 +2640,21 @@ status_t SurfaceFlinger::captureScreen(const sp<IBinder>& display,
// scheduled at this time, this will end-up being a no-op as well. // scheduled at this time, this will end-up being a no-op as well.
mEventQueue.invalidateTransactionNow(); 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, sp<MessageBase> msg = new MessageCaptureScreen(this,
display, producer, reqWidth, reqHeight, minLayerZ, maxLayerZ, display, IGraphicBufferProducer::asInterface( wrapper ),
isCpuConsumer); reqWidth, reqHeight, minLayerZ, maxLayerZ,
status_t res = postMessageSync(msg); useReadPixels);
status_t res = postMessageAsync(msg);
if (res == NO_ERROR) { if (res == NO_ERROR) {
res = static_cast<MessageCaptureScreen*>( msg.get() )->getResult(); res = wrapper->waitForResponse();
} }
return res; return res;
} }