From ebeb47000de33edd551d1d46fa0abe7100dbb30a Mon Sep 17 00:00:00 2001
From: Robert Shih <robertshih@google.com>
Date: Mon, 22 May 2017 17:32:29 -0700
Subject: [PATCH] DO NOT MERGE Don't leak `this` out of GraphicBufferSource
 ctor

Bug: 37622974
Bug: 37622987
Bug: 37623757
Test: run poc and observe no crash
Change-Id: I1e25c011f02bec26a1480ec9a217a52f15d43cf2
---
 media/libstagefright/omx/GraphicBufferSource.cpp | 12 +++++++-----
 media/libstagefright/omx/GraphicBufferSource.h   |  6 +-----
 media/libstagefright/omx/OMXNodeInstance.cpp     |  2 +-
 3 files changed, 9 insertions(+), 11 deletions(-)

diff --git a/media/libstagefright/omx/GraphicBufferSource.cpp b/media/libstagefright/omx/GraphicBufferSource.cpp
index 1a7dc9d9dd..bc7f52cfb0 100644
--- a/media/libstagefright/omx/GraphicBufferSource.cpp
+++ b/media/libstagefright/omx/GraphicBufferSource.cpp
@@ -170,9 +170,12 @@ GraphicBufferSource::GraphicBufferSource(
         mIsPersistent = true;
     }
     mConsumer->setDefaultBufferSize(bufferWidth, bufferHeight);
-    // Note that we can't create an sp<...>(this) in a ctor that will not keep a
-    // reference once the ctor ends, as that would cause the refcount of 'this'
-    // dropping to 0 at the end of the ctor.  Since all we need is a wp<...>
+}
+
+status_t GraphicBufferSource::init() {
+    // Note that we can't create an sp<...>(this) in a method that will not keep a
+    // reference once the method ends, as that may cause the refcount of 'this'
+    // dropping to 0 at the end of the method.  Since all we need is a wp<...>
     // that's what we create.
     wp<BufferQueue::ConsumerListener> listener = static_cast<BufferQueue::ConsumerListener*>(this);
     sp<IConsumerListener> proxy;
@@ -186,10 +189,9 @@ GraphicBufferSource::GraphicBufferSource(
     if (mInitCheck != NO_ERROR) {
         ALOGE("Error connecting to BufferQueue: %s (%d)",
                 strerror(-mInitCheck), mInitCheck);
-        return;
     }
 
-    CHECK(mInitCheck == NO_ERROR);
+    return mInitCheck;
 }
 
 GraphicBufferSource::~GraphicBufferSource() {
diff --git a/media/libstagefright/omx/GraphicBufferSource.h b/media/libstagefright/omx/GraphicBufferSource.h
index 2f929d95c7..b8e6c454c5 100644
--- a/media/libstagefright/omx/GraphicBufferSource.h
+++ b/media/libstagefright/omx/GraphicBufferSource.h
@@ -61,11 +61,7 @@ public:
 
     virtual ~GraphicBufferSource();
 
-    // We can't throw an exception if the constructor fails, so we just set
-    // this and require that the caller test the value.
-    status_t initCheck() const {
-        return mInitCheck;
-    }
+    status_t init();
 
     // Returns the handle to the producer side of the BufferQueue.  Buffers
     // queued on this will be received by GraphicBufferSource.
diff --git a/media/libstagefright/omx/OMXNodeInstance.cpp b/media/libstagefright/omx/OMXNodeInstance.cpp
index bb726e906e..e8765c7f14 100644
--- a/media/libstagefright/omx/OMXNodeInstance.cpp
+++ b/media/libstagefright/omx/OMXNodeInstance.cpp
@@ -1046,7 +1046,7 @@ status_t OMXNodeInstance::createGraphicBufferSource(
             usageBits,
             bufferConsumer);
 
-    if ((err = bufferSource->initCheck()) != OK) {
+    if ((err = bufferSource->init()) != OK) {
         return err;
     }
     setGraphicBufferSource(bufferSource);
-- 
2.11.0