From 673236b3a962335013baddee4065be138eb610b8 Mon Sep 17 00:00:00 2001 From: Dongwon Kang Date: Wed, 8 May 2019 09:02:21 -0700 Subject: [PATCH 1/2] Revert "Make tombstone in the child process on loading failure." This reverts commit e55d13cea8946ba51fa51d9e07e29bda141b12f8. Test: manualy added crash loop and check if it triggers rollback. Bug: 131106476 Change-Id: Ie49cec343467f58835f8ee01f72ecbedbadb4b79 --- media/codec2/vndk/C2Store.cpp | 29 +++++++------------ .../libstagefright/MediaExtractorFactory.cpp | 28 ++++++++---------- .../media/stagefright/foundation/ADebug.h | 9 ------ 3 files changed, 23 insertions(+), 43 deletions(-) diff --git a/media/codec2/vndk/C2Store.cpp b/media/codec2/vndk/C2Store.cpp index f5dc838f5e..10c4dcc574 100644 --- a/media/codec2/vndk/C2Store.cpp +++ b/media/codec2/vndk/C2Store.cpp @@ -26,7 +26,6 @@ #include #include #include -#include #include #include @@ -662,36 +661,30 @@ c2_status_t C2PlatformComponentStore::ComponentModule::init( ALOGV("in %s", __func__); ALOGV("loading dll"); mLibHandle = dlopen(libPath.c_str(), RTLD_NOW|RTLD_NODELETE); - if (mLibHandle == nullptr) { - LOG_ALWAYS_FATAL_IN_CHILD_PROC("could not dlopen %s: %s", libPath.c_str(), dlerror()); - mInit = C2_CORRUPTED; - return mInit; - } + LOG_ALWAYS_FATAL_IF(mLibHandle == nullptr, + "could not dlopen %s: %s", libPath.c_str(), dlerror()); createFactory = (C2ComponentFactory::CreateCodec2FactoryFunc)dlsym(mLibHandle, "CreateCodec2Factory"); - if (createFactory == nullptr) { - LOG_ALWAYS_FATAL_IN_CHILD_PROC("createFactory is null in %s", libPath.c_str()); - mInit = C2_CORRUPTED; - return mInit; - } + LOG_ALWAYS_FATAL_IF(createFactory == nullptr, + "createFactory is null in %s", libPath.c_str()); destroyFactory = (C2ComponentFactory::DestroyCodec2FactoryFunc)dlsym(mLibHandle, "DestroyCodec2Factory"); - if (destroyFactory == nullptr) { - LOG_ALWAYS_FATAL_IN_CHILD_PROC("destroyFactory is null in %s", libPath.c_str()); - mInit = C2_CORRUPTED; - return mInit; - } + LOG_ALWAYS_FATAL_IF(destroyFactory == nullptr, + "destroyFactory is null in %s", libPath.c_str()); mComponentFactory = createFactory(); if (mComponentFactory == nullptr) { ALOGD("could not create factory in %s", libPath.c_str()); mInit = C2_NO_MEMORY; - return mInit; + } else { + mInit = C2_OK; } - mInit = C2_OK; + if (mInit != C2_OK) { + return mInit; + } std::shared_ptr intf; c2_status_t res = createInterface(0, &intf); diff --git a/media/libstagefright/MediaExtractorFactory.cpp b/media/libstagefright/MediaExtractorFactory.cpp index 81d2abbced..d97591f8a3 100644 --- a/media/libstagefright/MediaExtractorFactory.cpp +++ b/media/libstagefright/MediaExtractorFactory.cpp @@ -19,11 +19,11 @@ #include #include +#include #include #include #include #include -#include #include #include #include @@ -245,21 +245,17 @@ void MediaExtractorFactory::RegisterExtractors( void *libHandle = android_dlopen_ext( libPath.string(), RTLD_NOW | RTLD_LOCAL, dlextinfo); - if (libHandle) { - GetExtractorDef getDef = - (GetExtractorDef) dlsym(libHandle, "GETEXTRACTORDEF"); - if (getDef) { - ALOGV("registering sniffer for %s", libPath.string()); - RegisterExtractor( - new ExtractorPlugin(getDef(), libHandle, libPath), pluginList); - } else { - LOG_ALWAYS_FATAL_IN_CHILD_PROC("%s does not contain sniffer", libPath.string()); - dlclose(libHandle); - } - } else { - LOG_ALWAYS_FATAL_IN_CHILD_PROC( - "couldn't dlopen(%s) %s", libPath.string(), strerror(errno)); - } + CHECK(libHandle != nullptr) + << "couldn't dlopen(" << libPath.string() << ") " << strerror(errno); + + GetExtractorDef getDef = + (GetExtractorDef) dlsym(libHandle, "GETEXTRACTORDEF"); + CHECK(getDef != nullptr) + << libPath.string() << " does not contain sniffer"; + + ALOGV("registering sniffer for %s", libPath.string()); + RegisterExtractor( + new ExtractorPlugin(getDef(), libHandle, libPath), pluginList); } closedir(libDir); } else { diff --git a/media/libstagefright/foundation/include/media/stagefright/foundation/ADebug.h b/media/libstagefright/foundation/include/media/stagefright/foundation/ADebug.h index 180694b4d4..a8b88fdd64 100644 --- a/media/libstagefright/foundation/include/media/stagefright/foundation/ADebug.h +++ b/media/libstagefright/foundation/include/media/stagefright/foundation/ADebug.h @@ -123,15 +123,6 @@ inline static const char *asString(status_t i, const char *def = "??") { #define TRESPASS_DBG(...) #endif -#ifndef LOG_ALWAYS_FATAL_IN_CHILD_PROC -#define LOG_ALWAYS_FATAL_IN_CHILD_PROC(...) \ - do { \ - if (fork() == 0) { \ - LOG_ALWAYS_FATAL(__VA_ARGS__); \ - } \ - } while (false) -#endif - struct ADebug { enum Level { kDebugNone, // no debug From c728a42d4f9f03f74b901fc7dd7fba1bc06579e9 Mon Sep 17 00:00:00 2001 From: Dongwon Kang Date: Wed, 8 May 2019 11:17:01 -0700 Subject: [PATCH 2/2] extractor: load extractors on boot, not on request. Test: build + dumpsys media.extractor Bug: 131106476 Change-Id: Idbea788340c4849f1286727c159a8eb43d6a7bb0 --- media/libstagefright/MediaExtractorFactory.cpp | 4 +--- .../include/media/stagefright/MediaExtractorFactory.h | 3 +-- services/mediaextractor/MediaExtractorService.cpp | 4 +++- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/media/libstagefright/MediaExtractorFactory.cpp b/media/libstagefright/MediaExtractorFactory.cpp index d97591f8a3..4c8be1f193 100644 --- a/media/libstagefright/MediaExtractorFactory.cpp +++ b/media/libstagefright/MediaExtractorFactory.cpp @@ -71,8 +71,6 @@ sp MediaExtractorFactory::CreateFromService( ALOGV("MediaExtractorFactory::CreateFromService %s", mime); - UpdateExtractors(); - // initialize source decryption if needed source->DrmInitialization(nullptr /* mime */); @@ -270,7 +268,7 @@ static bool compareFunc(const sp& first, const sp gSupportedExtensions; // static -void MediaExtractorFactory::UpdateExtractors() { +void MediaExtractorFactory::LoadExtractors() { Mutex::Autolock autoLock(gPluginMutex); if (gPluginsRegistered) { diff --git a/media/libstagefright/include/media/stagefright/MediaExtractorFactory.h b/media/libstagefright/include/media/stagefright/MediaExtractorFactory.h index ea87948d61..2ab98e1408 100644 --- a/media/libstagefright/include/media/stagefright/MediaExtractorFactory.h +++ b/media/libstagefright/include/media/stagefright/MediaExtractorFactory.h @@ -37,6 +37,7 @@ public: const sp &source, const char *mime = NULL); static status_t dump(int fd, const Vector& args); static std::unordered_set getSupportedTypes(); + static void LoadExtractors(); private: static Mutex gPluginMutex; @@ -53,8 +54,6 @@ private: static void *sniff(const sp &source, float *confidence, void **meta, FreeMetaFunc *freeMeta, sp &plugin, uint32_t *creatorVersion); - - static void UpdateExtractors(); }; } // namespace android diff --git a/services/mediaextractor/MediaExtractorService.cpp b/services/mediaextractor/MediaExtractorService.cpp index de5c3e4fef..36e084bc26 100644 --- a/services/mediaextractor/MediaExtractorService.cpp +++ b/services/mediaextractor/MediaExtractorService.cpp @@ -30,7 +30,9 @@ namespace android { MediaExtractorService::MediaExtractorService() - : BnMediaExtractorService() { } + : BnMediaExtractorService() { + MediaExtractorFactory::LoadExtractors(); +} sp MediaExtractorService::makeExtractor( const sp &remoteSource, const char *mime) {