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

Bug: 5679534

Change-Id: If447e8673df83fe0b1d6210641e0a48522501a53

diff --git a/include/gui/BufferQueue.h b/include/gui/BufferQueue.h
index 7e404fe..408956b 100644 (file)
--- a/include/gui/BufferQueue.h
+++ b/include/gui/BufferQueue.h
@@ -20,6 +20,8 @@
 #include <EGL/egl.h>
 #include <EGL/eglext.h>
 
+#include <binder/IBinder.h>
+
 #include <gui/IConsumerListener.h>
 #include <gui/IGraphicBufferAlloc.h>
 #include <gui/IGraphicBufferProducer.h>
@@ -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 };
@@ -79,6 +83,12 @@ public:
     virtual ~BufferQueue();
 
     /*
+     * IBinder::DeathRecipient interface
+     */
+
+    virtual void binderDied(const wp<IBinder>& 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<IBinder>& 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<IBinder> mConnectedProducerToken;
 };
 
 // ----------------------------------------------------------------------------

diff --git a/include/gui/IGraphicBufferProducer.h b/include/gui/IGraphicBufferProducer.h
index c3ede5e..342ba08 100644 (file)
--- 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<IBinder>& 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 57a41f2..50e3079 100644 (file)
--- a/libs/gui/BufferQueue.cpp
+++ b/libs/gui/BufferQueue.cpp
@@ -635,7 +635,9 @@ void BufferQueue::cancelBuffer(int buf, const sp<Fence>& fence) {
     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();
     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<IBinder::DeathRecipient*>(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<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) {
     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<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;
                     mDequeueCondition.broadcast();
                     listener = mConsumerListener;

diff --git a/libs/gui/IGraphicBufferProducer.cpp b/libs/gui/IGraphicBufferProducer.cpp
index 3080220..fc86e60 100644 (file)
--- a/libs/gui/IGraphicBufferProducer.cpp
+++ b/libs/gui/IGraphicBufferProducer.cpp
@@ -41,7 +41,6 @@ enum {
     DISCONNECT,
 };
 
-
 class BpGraphicBufferProducer : public BpInterface<IGraphicBufferProducer>
 {
 public:
@@ -139,9 +138,11 @@ public:
         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;
         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<IBinder> token = data.readStrongBinder();
             int api = data.readInt32();
             bool producerControlledByApp = data.readInt32();
             QueueBufferOutput* const output =
                     reinterpret_cast<QueueBufferOutput *>(
                             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 1bae0fe..27dbc4e 100644 (file)
--- 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<BBinder> 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 43d27bb..c06043d 100644 (file)
--- 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<IBinder>& 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 536007e..18fb5a7 100644 (file)
--- 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>& fence);
     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);
 
     //