From 365857df8b94c959dea984a63013f6e7730ef976 Mon Sep 17 00:00:00 2001 From: Mathias Agopian Date: Wed, 11 Sep 2013 19:35:45 -0700 Subject: [PATCH] Make sure do disconnect from a BQ when its client dies. Bug: 5679534 Change-Id: If447e8673df83fe0b1d6210641e0a48522501a53 --- include/gui/BufferQueue.h | 18 +++++++++-- include/gui/IGraphicBufferProducer.h | 7 ++-- libs/gui/BufferQueue.cpp | 32 +++++++++++++++++-- libs/gui/IGraphicBufferProducer.cpp | 8 +++-- libs/gui/Surface.cpp | 4 ++- .../DisplayHardware/VirtualDisplaySurface.cpp | 5 +-- .../DisplayHardware/VirtualDisplaySurface.h | 3 +- 7 files changed, 63 insertions(+), 14 deletions(-) diff --git a/include/gui/BufferQueue.h b/include/gui/BufferQueue.h index 7e404fe84..408956b6e 100644 --- a/include/gui/BufferQueue.h +++ b/include/gui/BufferQueue.h @@ -20,6 +20,8 @@ #include #include +#include + #include #include #include @@ -35,7 +37,9 @@ namespace android { // ---------------------------------------------------------------------------- -class BufferQueue : public BnGraphicBufferProducer, public BnGraphicBufferConsumer { +class BufferQueue : public BnGraphicBufferProducer, + public BnGraphicBufferConsumer, + private IBinder::DeathRecipient { public: enum { MIN_UNDEQUEUED_BUFFERS = 2 }; enum { NUM_BUFFER_SLOTS = 32 }; @@ -78,6 +82,12 @@ public: BufferQueue(const sp& allocator = NULL); virtual ~BufferQueue(); + /* + * IBinder::DeathRecipient interface + */ + + virtual void binderDied(const wp& who); + /* * IGraphicBufferProducer interface */ @@ -184,7 +194,8 @@ public: // it's still connected to a producer). // // 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& token, + int api, bool producerControlledByApp, QueueBufferOutput* output); // disconnect attempts to disconnect a producer API from the BufferQueue. // Calling this method will cause any subsequent calls to other @@ -552,6 +563,9 @@ private: // mTransformHint is used to optimize for screen rotations uint32_t mTransformHint; + + // mConnectedProducerToken is used to set a binder death notification on the producer + sp mConnectedProducerToken; }; // ---------------------------------------------------------------------------- diff --git a/include/gui/IGraphicBufferProducer.h b/include/gui/IGraphicBufferProducer.h index c3ede5ef7..342ba0811 100644 --- a/include/gui/IGraphicBufferProducer.h +++ b/include/gui/IGraphicBufferProducer.h @@ -189,8 +189,11 @@ public: // // outWidth, outHeight and outTransform are filled with the default width // and height of the window and current transform applied to buffers, - // respectively. - virtual status_t connect(int api, bool producerControlledByApp, QueueBufferOutput* output) = 0; + // respectively. The token needs to be any binder object that lives in the + // producer process -- it is solely used for obtaining a death notification + // when the producer is killed. + virtual status_t connect(const sp& token, + int api, bool producerControlledByApp, QueueBufferOutput* output) = 0; // disconnect attempts to disconnect a client API from the // IGraphicBufferProducer. Calling this method will cause any subsequent diff --git a/libs/gui/BufferQueue.cpp b/libs/gui/BufferQueue.cpp index 57a41f25c..50e307926 100644 --- a/libs/gui/BufferQueue.cpp +++ b/libs/gui/BufferQueue.cpp @@ -635,7 +635,9 @@ void BufferQueue::cancelBuffer(int buf, const sp& fence) { mDequeueCondition.broadcast(); } -status_t BufferQueue::connect(int api, bool producerControlledByApp, QueueBufferOutput* output) { + +status_t BufferQueue::connect(const sp& token, + int api, bool producerControlledByApp, QueueBufferOutput* output) { ATRACE_CALL(); ST_LOGV("connect: api=%d producerControlledByApp=%s", api, producerControlledByApp ? "true" : "false"); @@ -663,8 +665,14 @@ status_t BufferQueue::connect(int api, bool producerControlledByApp, QueueBuffer err = -EINVAL; } else { mConnectedApi = api; - output->inflate(mDefaultWidth, mDefaultHeight, mTransformHint, - mQueue.size()); + output->inflate(mDefaultWidth, mDefaultHeight, mTransformHint, 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(this)) == NO_ERROR) { + mConnectedProducerToken = token; + } } break; default: @@ -678,6 +686,16 @@ status_t BufferQueue::connect(int api, bool producerControlledByApp, QueueBuffer return err; } +void BufferQueue::binderDied(const wp& 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) { ATRACE_CALL(); ST_LOGV("disconnect: api=%d", api); @@ -701,6 +719,14 @@ status_t BufferQueue::disconnect(int api) { case NATIVE_WINDOW_API_CAMERA: if (mConnectedApi == api) { freeAllBuffersLocked(); + // remove our death notification callback if we have one + sp 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(this)); + } + mConnectedProducerToken = NULL; mConnectedApi = NO_CONNECTED_API; mDequeueCondition.broadcast(); listener = mConsumerListener; diff --git a/libs/gui/IGraphicBufferProducer.cpp b/libs/gui/IGraphicBufferProducer.cpp index 3080220f4..fc86e608a 100644 --- a/libs/gui/IGraphicBufferProducer.cpp +++ b/libs/gui/IGraphicBufferProducer.cpp @@ -41,7 +41,6 @@ enum { DISCONNECT, }; - class BpGraphicBufferProducer : public BpInterface { public: @@ -139,9 +138,11 @@ public: return result; } - virtual status_t connect(int api, bool producerControlledByApp, QueueBufferOutput* output) { + virtual status_t connect(const sp& token, + int api, bool producerControlledByApp, QueueBufferOutput* output) { Parcel data, reply; data.writeInterfaceToken(IGraphicBufferProducer::getInterfaceDescriptor()); + data.writeStrongBinder(token); data.writeInt32(api); data.writeInt32(producerControlledByApp); status_t result = remote()->transact(CONNECT, data, &reply); @@ -241,12 +242,13 @@ status_t BnGraphicBufferProducer::onTransact( } break; case CONNECT: { CHECK_INTERFACE(IGraphicBufferProducer, data, reply); + sp token = data.readStrongBinder(); int api = data.readInt32(); bool producerControlledByApp = data.readInt32(); QueueBufferOutput* const output = reinterpret_cast( reply->writeInplace(sizeof(QueueBufferOutput))); - status_t res = connect(api, producerControlledByApp, output); + status_t res = connect(token, api, producerControlledByApp, output); reply->writeInt32(res); return NO_ERROR; } break; diff --git a/libs/gui/Surface.cpp b/libs/gui/Surface.cpp index 1bae0fed4..27dbc4eee 100644 --- a/libs/gui/Surface.cpp +++ b/libs/gui/Surface.cpp @@ -490,9 +490,10 @@ int Surface::dispatchUnlockAndPost(va_list args) { int Surface::connect(int api) { ATRACE_CALL(); ALOGV("Surface::connect"); + static sp sLife = new BBinder(); Mutex::Autolock lock(mMutex); IGraphicBufferProducer::QueueBufferOutput output; - int err = mGraphicBufferProducer->connect(api, mProducerControlledByApp, &output); + int err = mGraphicBufferProducer->connect(sLife, api, mProducerControlledByApp, &output); if (err == NO_ERROR) { uint32_t numPendingBuffers = 0; output.deflate(&mDefaultWidth, &mDefaultHeight, &mTransformHint, @@ -505,6 +506,7 @@ int Surface::connect(int api) { return err; } + int Surface::disconnect(int api) { ATRACE_CALL(); ALOGV("Surface::disconnect"); diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp index 43d27bb24..c06043d4f 100644 --- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp +++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.cpp @@ -358,10 +358,11 @@ int VirtualDisplaySurface::query(int what, int* value) { return mSource[SOURCE_SINK]->query(what, value); } -status_t VirtualDisplaySurface::connect(int api, bool producerControlledByApp, +status_t VirtualDisplaySurface::connect(const sp& token, + int api, bool producerControlledByApp, QueueBufferOutput* output) { 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) { updateQueueBufferOutput(qbo); *output = mQueueBufferOutput; diff --git a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h index 536007ec6..18fb5a7e0 100644 --- a/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h +++ b/services/surfaceflinger/DisplayHardware/VirtualDisplaySurface.h @@ -102,7 +102,8 @@ private: const QueueBufferInput& input, QueueBufferOutput* output); virtual void cancelBuffer(int pslot, const sp& fence); virtual int query(int what, int* value); - virtual status_t connect(int api, bool producerControlledByApp, QueueBufferOutput* output); + virtual status_t connect(const sp& token, + int api, bool producerControlledByApp, QueueBufferOutput* output); virtual status_t disconnect(int api); //