NuCachedSource2: fix possible erroneous early free

Because the constructor of NuCachedSource2 sent a message to
AHandlerReflector object, AHandlerReflector::onMessageReceived could
have executed just before the object gets wrapped in a strong
pointer, resulting in erroneous early free. Fix the issue by using
static Create function to ensure the message is sent after the
object is wrapped in a sp.

Bug: 23882800
Change-Id: I38a9d7a3083f184b4c81d0b00ba1661721278855

4 files changed, 21 insertions, 7 deletions

diff --git a/media/libstagefright/AwesomePlayer.cpp b/media/libstagefright/AwesomePlayer.cpp
index 4e6c2a6..3cd0b0e 100644
--- a/media/libstagefright/AwesomePlayer.cpp
+++ b/media/libstagefright/AwesomePlayer.cpp
@@ -2290,11 +2290,11 @@ status_t AwesomePlayer::finishSetDataSource_l() {
             // The widevine extractor does its own caching.
 
 #if 0
-            mCachedSource = new NuCachedSource2(
+            mCachedSource = NuCachedSource2::Create(
                     new ThrottledSource(
                         mConnectingDataSource, 50 * 1024 /* bytes/sec */));
 #else
-            mCachedSource = new NuCachedSource2(
+            mCachedSource = NuCachedSource2::Create(
                     mConnectingDataSource,
                     cacheConfig.isEmpty() ? NULL : cacheConfig.string(),
                     disconnectAtHighwatermark);
diff --git a/media/libstagefright/DataSource.cpp b/media/libstagefright/DataSource.cpp
index 75ef288..5020c6c 100644
--- a/media/libstagefright/DataSource.cpp
+++ b/media/libstagefright/DataSource.cpp
@@ -246,7 +246,7 @@ sp<DataSource> DataSource::CreateFromURI(
                 *contentType = httpSource->getMIMEType();
             }
 
-            source = new NuCachedSource2(
+            source = NuCachedSource2::Create(
                     httpSource,
                     cacheConfig.isEmpty() ? NULL : cacheConfig.string(),
                     disconnectAtHighwatermark);
diff --git a/media/libstagefright/NuCachedSource2.cpp b/media/libstagefright/NuCachedSource2.cpp
index f82636b..d6255d6 100644
--- a/media/libstagefright/NuCachedSource2.cpp
+++ b/media/libstagefright/NuCachedSource2.cpp
@@ -224,9 +224,6 @@ NuCachedSource2::NuCachedSource2(
     // So whenever we call DataSource::readAt it may end up in a call to
     // IMediaHTTPConnection::readAt and therefore call back into JAVA.
     mLooper->start(false /* runOnCallingThread */, true /* canCallJava */);
-
-    Mutex::Autolock autoLock(mLock);
-    (new AMessage(kWhatFetchMore, mReflector))->post();
 }
 
 NuCachedSource2::~NuCachedSource2() {
@@ -237,6 +234,18 @@ NuCachedSource2::~NuCachedSource2() {
     mCache = NULL;
 }
 
+// static
+sp<NuCachedSource2> NuCachedSource2::Create(
+        const sp<DataSource> &source,
+        const char *cacheConfig,
+        bool disconnectAtHighwatermark) {
+    sp<NuCachedSource2> instance = new NuCachedSource2(
+            source, cacheConfig, disconnectAtHighwatermark);
+    Mutex::Autolock autoLock(instance->mLock);
+    (new AMessage(kWhatFetchMore, instance->mReflector))->post();
+    return instance;
+}
+
 status_t NuCachedSource2::getEstimatedBandwidthKbps(int32_t *kbps) {
     if (mSource->flags() & kIsHTTPBasedSource) {
         HTTPBase* source = static_cast<HTTPBase *>(mSource.get());
diff --git a/media/libstagefright/include/NuCachedSource2.h b/media/libstagefright/include/NuCachedSource2.h
index 4252706..a29bdf9 100644
--- a/media/libstagefright/include/NuCachedSource2.h
+++ b/media/libstagefright/include/NuCachedSource2.h
@@ -28,7 +28,7 @@ struct ALooper;
 struct PageCache;
 
 struct NuCachedSource2 : public DataSource {
-    NuCachedSource2(
+    static sp<NuCachedSource2> Create(
             const sp<DataSource> &source,
             const char *cacheConfig = NULL,
             bool disconnectAtHighwatermark = false);
@@ -72,6 +72,11 @@ protected:
 private:
     friend struct AHandlerReflector<NuCachedSource2>;
 
+    NuCachedSource2(
+            const sp<DataSource> &source,
+            const char *cacheConfig,
+            bool disconnectAtHighwatermark);
+
     enum {
         kPageSize                       = 65536,
         kDefaultHighWaterThreshold      = 20 * 1024 * 1024,