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
(cherry picked from commit 6301f882512ec39baf28640f31b90104def1738d)
diff --git a/media/libstagefright/omx/GraphicBufferSource.cpp b/media/libstagefright/omx/GraphicBufferSource.cpp
index 0c4056d..3bcc599 100644
--- a/media/libstagefright/omx/GraphicBufferSource.cpp
+++ b/media/libstagefright/omx/GraphicBufferSource.cpp
@@ -177,9 +177,12 @@
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;
@@ -190,15 +193,14 @@
}
mInitCheck = mConsumer->consumerConnect(proxy, false);
- if (mInitCheck != NO_ERROR) {
+ if (mInitCheck == NO_ERROR) {
+ memset(&mColorAspects, 0, sizeof(mColorAspects));
+ } else {
ALOGE("Error connecting to BufferQueue: %s (%d)",
strerror(-mInitCheck), mInitCheck);
- return;
}
- memset(&mColorAspects, 0, sizeof(mColorAspects));
-
- CHECK(mInitCheck == NO_ERROR);
+ return mInitCheck;
}
GraphicBufferSource::~GraphicBufferSource() {
diff --git a/media/libstagefright/omx/GraphicBufferSource.h b/media/libstagefright/omx/GraphicBufferSource.h
index aa4ceb3..b3fcd1b 100644
--- a/media/libstagefright/omx/GraphicBufferSource.h
+++ b/media/libstagefright/omx/GraphicBufferSource.h
@@ -62,11 +62,7 @@
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 c3514b3..e7aaead 100644
--- a/media/libstagefright/omx/OMXNodeInstance.cpp
+++ b/media/libstagefright/omx/OMXNodeInstance.cpp
@@ -1130,7 +1130,7 @@
usageBits,
bufferConsumer);
- if ((err = bufferSource->initCheck()) != OK) {
+ if ((err = bufferSource->init()) != OK) {
return err;
}
setGraphicBufferSource(bufferSource);
