Make sure do disconnect from a BQ when its client dies.

Bug: 5679534

Change-Id: If447e8673df83fe0b1d6210641e0a48522501a53
This commit is contained in:
Mathias Agopian 2013-09-11 19:35:45 -07:00
parent 90ed3e8d78
commit 365857df8b
7 changed files with 63 additions and 14 deletions

View File

@ -20,6 +20,8 @@
#include <EGL/egl.h> #include <EGL/egl.h>
#include <EGL/eglext.h> #include <EGL/eglext.h>
#include <binder/IBinder.h>
#include <gui/IConsumerListener.h> #include <gui/IConsumerListener.h>
#include <gui/IGraphicBufferAlloc.h> #include <gui/IGraphicBufferAlloc.h>
#include <gui/IGraphicBufferProducer.h> #include <gui/IGraphicBufferProducer.h>
@ -35,7 +37,9 @@
namespace android { namespace android {
// ---------------------------------------------------------------------------- // ----------------------------------------------------------------------------
class BufferQueue : public BnGraphicBufferProducer, public BnGraphicBufferConsumer { class BufferQueue : public BnGraphicBufferProducer,
public BnGraphicBufferConsumer,
private IBinder::DeathRecipient {
public: public:
enum { MIN_UNDEQUEUED_BUFFERS = 2 }; enum { MIN_UNDEQUEUED_BUFFERS = 2 };
enum { NUM_BUFFER_SLOTS = 32 }; enum { NUM_BUFFER_SLOTS = 32 };
@ -78,6 +82,12 @@ public:
BufferQueue(const sp<IGraphicBufferAlloc>& allocator = NULL); BufferQueue(const sp<IGraphicBufferAlloc>& allocator = NULL);
virtual ~BufferQueue(); virtual ~BufferQueue();
/*
* IBinder::DeathRecipient interface
*/
virtual void binderDied(const wp<IBinder>& who);
/* /*
* IGraphicBufferProducer interface * IGraphicBufferProducer interface
*/ */
@ -184,7 +194,8 @@ public:
// it's still connected to a producer). // it's still connected to a producer).
// //
// APIs are enumerated in window.h (e.g. NATIVE_WINDOW_API_CPU). // APIs are enumerated in window.h (e.g. NATIVE_WINDOW_API_CPU).
virtual status_t connect(int api, bool producerControlledByApp, QueueBufferOutput* output); virtual status_t connect(const sp<IBinder>& token,
int api, bool producerControlledByApp, QueueBufferOutput* output);
// disconnect attempts to disconnect a producer API from the BufferQueue. // disconnect attempts to disconnect a producer API from the BufferQueue.
// Calling this method will cause any subsequent calls to other // Calling this method will cause any subsequent calls to other
@ -552,6 +563,9 @@ private:
// mTransformHint is used to optimize for screen rotations // mTransformHint is used to optimize for screen rotations
uint32_t mTransformHint; uint32_t mTransformHint;
// mConnectedProducerToken is used to set a binder death notification on the producer
sp<IBinder> mConnectedProducerToken;
}; };
// ---------------------------------------------------------------------------- // ----------------------------------------------------------------------------

View File

@ -189,8 +189,11 @@ public:
// //
// outWidth, outHeight and outTransform are filled with the default width // outWidth, outHeight and outTransform are filled with the default width
// and height of the window and current transform applied to buffers, // and height of the window and current transform applied to buffers,
// respectively. // respectively. The token needs to be any binder object that lives in the
virtual status_t connect(int api, bool producerControlledByApp, QueueBufferOutput* output) = 0; // producer process -- it is solely used for obtaining a death notification
// when the producer is killed.
virtual status_t connect(const sp<IBinder>& token,
int api, bool producerControlledByApp, QueueBufferOutput* output) = 0;
// disconnect attempts to disconnect a client API from the // disconnect attempts to disconnect a client API from the
// IGraphicBufferProducer. Calling this method will cause any subsequent // IGraphicBufferProducer. Calling this method will cause any subsequent

View File

@ -635,7 +635,9 @@ void BufferQueue::cancelBuffer(int buf, const sp<Fence>& fence) {
mDequeueCondition.broadcast(); mDequeueCondition.broadcast();
} }
status_t BufferQueue::connect(int api, bool producerControlledByApp, QueueBufferOutput* output) {
status_t BufferQueue::connect(const sp<IBinder>& token,
int api, bool producerControlledByApp, QueueBufferOutput* output) {
ATRACE_CALL(); ATRACE_CALL();
ST_LOGV("connect: api=%d producerControlledByApp=%s", api, ST_LOGV("connect: api=%d producerControlledByApp=%s", api,
producerControlledByApp ? "true" : "false"); producerControlledByApp ? "true" : "false");
@ -663,8 +665,14 @@ status_t BufferQueue::connect(int api, bool producerControlledByApp, QueueBuffer
err = -EINVAL; err = -EINVAL;
} else { } else {
mConnectedApi = api; mConnectedApi = api;
output->inflate(mDefaultWidth, mDefaultHeight, mTransformHint, output->inflate(mDefaultWidth, mDefaultHeight, mTransformHint, mQueue.size());
mQueue.size());
// set-up a death notification so that we can disconnect automatically
// when/if the remote producer dies.
// This will fail with INVALID_OPERATION if the "token" is local to our process.
if (token->linkToDeath(static_cast<IBinder::DeathRecipient*>(this)) == NO_ERROR) {
mConnectedProducerToken = token;
}
} }
break; break;
default: default:
@ -678,6 +686,16 @@ status_t BufferQueue::connect(int api, bool producerControlledByApp, QueueBuffer
return err; return err;
} }
void BufferQueue::binderDied(const wp<IBinder>& who) {
// If we're here, it means that a producer we were connected to died.
// We're GUARANTEED that we still are connected to it because it has no other way
// to get disconnected -- or -- we wouldn't be here because we're removing this
// callback upon disconnect. Therefore, it's okay to read mConnectedApi without
// synchronization here.
int api = mConnectedApi;
this->disconnect(api);
}
status_t BufferQueue::disconnect(int api) { status_t BufferQueue::disconnect(int api) {
ATRACE_CALL(); ATRACE_CALL();
ST_LOGV("disconnect: api=%d", api); ST_LOGV("disconnect: api=%d", api);
@ -701,6 +719,14 @@ status_t BufferQueue::disconnect(int api) {
case NATIVE_WINDOW_API_CAMERA: case NATIVE_WINDOW_API_CAMERA:
if (mConnectedApi == api) { if (mConnectedApi == api) {
freeAllBuffersLocked(); freeAllBuffersLocked();
// remove our death notification callback if we have one
sp<IBinder> token = mConnectedProducerToken;
if (token != NULL) {
// this can fail if we're here because of the death notification
// either way, we just ignore.
token->unlinkToDeath(static_cast<IBinder::DeathRecipient*>(this));
}
mConnectedProducerToken = NULL;
mConnectedApi = NO_CONNECTED_API; mConnectedApi = NO_CONNECTED_API;
mDequeueCondition.broadcast(); mDequeueCondition.broadcast();
listener = mConsumerListener; listener = mConsumerListener;

View File

@ -41,7 +41,6 @@ enum {
DISCONNECT, DISCONNECT,
}; };
class BpGraphicBufferProducer : public BpInterface<IGraphicBufferProducer> class BpGraphicBufferProducer : public BpInterface<IGraphicBufferProducer>
{ {
public: public:
@ -139,9 +138,11 @@ public:
return result; return result;
} }
virtual status_t connect(int api, bool producerControlledByApp, QueueBufferOutput* output) { virtual status_t connect(const sp<IBinder>& token,
int api, bool producerControlledByApp, QueueBufferOutput* output) {
Parcel data, reply; Parcel data, reply;
data.writeInterfaceToken(IGraphicBufferProducer::getInterfaceDescriptor()); data.writeInterfaceToken(IGraphicBufferProducer::getInterfaceDescriptor());
data.writeStrongBinder(token);
data.writeInt32(api); data.writeInt32(api);
data.writeInt32(producerControlledByApp); data.writeInt32(producerControlledByApp);
status_t result = remote()->transact(CONNECT, data, &reply); status_t result = remote()->transact(CONNECT, data, &reply);
@ -241,12 +242,13 @@ status_t BnGraphicBufferProducer::onTransact(
} break; } break;
case CONNECT: { case CONNECT: {
CHECK_INTERFACE(IGraphicBufferProducer, data, reply); CHECK_INTERFACE(IGraphicBufferProducer, data, reply);
sp<IBinder> token = data.readStrongBinder();
int api = data.readInt32(); int api = data.readInt32();
bool producerControlledByApp = data.readInt32(); bool producerControlledByApp = data.readInt32();
QueueBufferOutput* const output = QueueBufferOutput* const output =
reinterpret_cast<QueueBufferOutput *>( reinterpret_cast<QueueBufferOutput *>(
reply->writeInplace(sizeof(QueueBufferOutput))); reply->writeInplace(sizeof(QueueBufferOutput)));
status_t res = connect(api, producerControlledByApp, output); status_t res = connect(token, api, producerControlledByApp, output);
reply->writeInt32(res); reply->writeInt32(res);
return NO_ERROR; return NO_ERROR;
} break; } break;

View File

@ -490,9 +490,10 @@ int Surface::dispatchUnlockAndPost(va_list args) {
int Surface::connect(int api) { int Surface::connect(int api) {
ATRACE_CALL(); ATRACE_CALL();
ALOGV("Surface::connect"); ALOGV("Surface::connect");
static sp<BBinder> sLife = new BBinder();
Mutex::Autolock lock(mMutex); Mutex::Autolock lock(mMutex);
IGraphicBufferProducer::QueueBufferOutput output; IGraphicBufferProducer::QueueBufferOutput output;
int err = mGraphicBufferProducer->connect(api, mProducerControlledByApp, &output); int err = mGraphicBufferProducer->connect(sLife, api, mProducerControlledByApp, &output);
if (err == NO_ERROR) { if (err == NO_ERROR) {
uint32_t numPendingBuffers = 0; uint32_t numPendingBuffers = 0;
output.deflate(&mDefaultWidth, &mDefaultHeight, &mTransformHint, output.deflate(&mDefaultWidth, &mDefaultHeight, &mTransformHint,
@ -505,6 +506,7 @@ int Surface::connect(int api) {
return err; return err;
} }
int Surface::disconnect(int api) { int Surface::disconnect(int api) {
ATRACE_CALL(); ATRACE_CALL();
ALOGV("Surface::disconnect"); ALOGV("Surface::disconnect");

View File

@ -358,10 +358,11 @@ int VirtualDisplaySurface::query(int what, int* value) {
return mSource[SOURCE_SINK]->query(what, value); return mSource[SOURCE_SINK]->query(what, value);
} }
status_t VirtualDisplaySurface::connect(int api, bool producerControlledByApp, status_t VirtualDisplaySurface::connect(const sp<IBinder>& token,
int api, bool producerControlledByApp,
QueueBufferOutput* output) { QueueBufferOutput* output) {
QueueBufferOutput qbo; QueueBufferOutput qbo;
status_t result = mSource[SOURCE_SINK]->connect(api, producerControlledByApp, &qbo); status_t result = mSource[SOURCE_SINK]->connect(token, api, producerControlledByApp, &qbo);
if (result == NO_ERROR) { if (result == NO_ERROR) {
updateQueueBufferOutput(qbo); updateQueueBufferOutput(qbo);
*output = mQueueBufferOutput; *output = mQueueBufferOutput;

View File

@ -102,7 +102,8 @@ private:
const QueueBufferInput& input, QueueBufferOutput* output); const QueueBufferInput& input, QueueBufferOutput* output);
virtual void cancelBuffer(int pslot, const sp<Fence>& fence); virtual void cancelBuffer(int pslot, const sp<Fence>& fence);
virtual int query(int what, int* value); virtual int query(int what, int* value);
virtual status_t connect(int api, bool producerControlledByApp, QueueBufferOutput* output); virtual status_t connect(const sp<IBinder>& token,
int api, bool producerControlledByApp, QueueBufferOutput* output);
virtual status_t disconnect(int api); virtual status_t disconnect(int api);
// //