From 3e937eac2b2cb22673ac0824123bf6881b4fbac3 Mon Sep 17 00:00:00 2001 From: Dratwas Date: Mon, 3 Jun 2019 07:08:42 -0700 Subject: [PATCH] fix indexed RAM bundle (#24967) Summary: Co-Authored: zamotany With React Native 0.59.8 the app keeps crashing with indexed RAM bundle on Android with the following error: ``` 2019-05-09 11:58:06.684 2793-2856/? E/AndroidRuntime: FATAL EXCEPTION: mqt_js Process: com.ramtestapp, PID: 2793 com.facebook.jni.CppException: getPropertyAsObject: property '__fbRequireBatchedBridge' is not an Object no stack at com.facebook.react.bridge.queue.NativeRunnable.run(Native Method) at android.os.Handler.handleCallback(Handler.java:873) at android.os.Handler.dispatchMessage(Handler.java:99) at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:29) at android.os.Looper.loop(Looper.java:193) at com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run(MessageQueueThreadImpl.java:232) at java.lang.Thread.run(Thread.java:764) ``` After investigation we found that when using any bundle, let it be non-ram, FIle RAM bundle or Index RAM bundle, the `CatalystInstanceImpl.java` is always using `loadScriptsFromAsset`, which is calling `CatalystInstanceImpl::jniLoadScriptFromAssets` in C++. This method when checking if bundle is a RAM bundle, uses `JniJSModulesUnbundle::isUnbundle` which only check for js-modules/UNBUNDLE - file generated when building File RAM bundle. There is no other logic to handle Indexed RAM bundle, so it figures that the bundle is not RAM, cause there is no js-modules/UNBUNDLE file and tries to load as regular bundle and fails. In this PR we added check if it is indexed RAM bundle in `jniLoadScriptFromAssets` and handle it if it is. ## Changelog [Android] [Fixed] fix indexed RAM bundle Solves https://github.com/facebook/react-native/issues/21282 Pull Request resolved: https://github.com/facebook/react-native/pull/24967 Differential Revision: D15575924 Pulled By: cpojer fbshipit-source-id: 5ea428e0b793edd8242243f39f933d1092b35260 --- .../jni/react/jni/CatalystInstanceImpl.cpp | 2 ++ ReactCommon/cxxreact/Instance.cpp | 18 ++++++++++ ReactCommon/cxxreact/Instance.h | 2 ++ ReactCommon/cxxreact/JSIndexedRAMBundle.cpp | 35 ++++++++++++++----- ReactCommon/cxxreact/JSIndexedRAMBundle.h | 8 +++-- 5 files changed, 53 insertions(+), 12 deletions(-) diff --git a/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp b/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp index 06a224f457a..28de85f7ba2 100644 --- a/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp +++ b/ReactAndroid/src/main/jni/react/jni/CatalystInstanceImpl.cpp @@ -199,6 +199,8 @@ void CatalystInstanceImpl::jniLoadScriptFromAssets( sourceURL, loadSynchronously); return; + } else if (Instance::isIndexedRAMBundle(&script)) { + instance_->loadRAMBundleFromString(std::move(script), sourceURL); } else { instance_->loadScriptFromString(std::move(script), sourceURL, loadSynchronously); } diff --git a/ReactCommon/cxxreact/Instance.cpp b/ReactCommon/cxxreact/Instance.cpp index ad5069238d6..81996813893 100644 --- a/ReactCommon/cxxreact/Instance.cpp +++ b/ReactCommon/cxxreact/Instance.cpp @@ -108,6 +108,24 @@ bool Instance::isIndexedRAMBundle(const char *sourcePath) { return parseTypeFromHeader(header) == ScriptTag::RAMBundle; } +bool Instance::isIndexedRAMBundle(std::unique_ptr* script) { + BundleHeader header; + strncpy(reinterpret_cast(&header), script->get()->c_str(), sizeof(header)); + + return parseTypeFromHeader(header) == ScriptTag::RAMBundle; +} + +void Instance::loadRAMBundleFromString(std::unique_ptr script, const std::string& sourceURL) { + auto bundle = folly::make_unique(std::move(script)); + auto startupScript = bundle->getStartupCode(); + auto registry = RAMBundleRegistry::singleBundleRegistry(std::move(bundle)); + loadRAMBundle( + std::move(registry), + std::move(startupScript), + sourceURL, + true); +} + void Instance::loadRAMBundleFromFile(const std::string& sourcePath, const std::string& sourceURL, bool loadSynchronously) { diff --git a/ReactCommon/cxxreact/Instance.h b/ReactCommon/cxxreact/Instance.h index b72729660ff..b129ee7e633 100644 --- a/ReactCommon/cxxreact/Instance.h +++ b/ReactCommon/cxxreact/Instance.h @@ -47,6 +47,8 @@ public: void loadScriptFromString(std::unique_ptr string, std::string sourceURL, bool loadSynchronously); static bool isIndexedRAMBundle(const char *sourcePath); + static bool isIndexedRAMBundle(std::unique_ptr* string); + void loadRAMBundleFromString(std::unique_ptr script, const std::string& sourceURL); void loadRAMBundleFromFile(const std::string& sourcePath, const std::string& sourceURL, bool loadSynchronously); diff --git a/ReactCommon/cxxreact/JSIndexedRAMBundle.cpp b/ReactCommon/cxxreact/JSIndexedRAMBundle.cpp index 6c2e7e87c7e..65c73a11aef 100644 --- a/ReactCommon/cxxreact/JSIndexedRAMBundle.cpp +++ b/ReactCommon/cxxreact/JSIndexedRAMBundle.cpp @@ -6,7 +6,8 @@ #include "JSIndexedRAMBundle.h" #include - +#include +#include #include namespace facebook { @@ -18,14 +19,30 @@ std::function(std::string)> JSIndexedRAMBundl }; } -JSIndexedRAMBundle::JSIndexedRAMBundle(const char *sourcePath) : - m_bundle (sourcePath, std::ios_base::in) { +JSIndexedRAMBundle::JSIndexedRAMBundle(const char *sourcePath) { + m_bundle = std::make_unique(sourcePath, std::ifstream::binary); if (!m_bundle) { throw std::ios_base::failure( folly::to("Bundle ", sourcePath, - "cannot be opened: ", m_bundle.rdstate())); + "cannot be opened: ", m_bundle->rdstate())); } + init(); +} +JSIndexedRAMBundle::JSIndexedRAMBundle(std::unique_ptr script) { + // tmpStream is needed because m_bundle is std::istream type + // which has no member 'write' + std::unique_ptr tmpStream = std::make_unique(); + tmpStream->write(script->c_str(), script->size()); + m_bundle = std::move(tmpStream); + if (!m_bundle) { + throw std::ios_base::failure( + folly::to("Bundle from string cannot be opened: ", m_bundle->rdstate())); + } + init(); +} + +void JSIndexedRAMBundle::init() { // read in magic header, number of entries, and length of the startup section uint32_t header[3]; static_assert( @@ -78,12 +95,12 @@ std::string JSIndexedRAMBundle::getModuleCode(const uint32_t id) const { } void JSIndexedRAMBundle::readBundle(char *buffer, const std::streamsize bytes) const { - if (!m_bundle.read(buffer, bytes)) { - if (m_bundle.rdstate() & std::ios::eofbit) { + if (!m_bundle->read(buffer, bytes)) { + if (m_bundle->rdstate() & std::ios::eofbit) { throw std::ios_base::failure("Unexpected end of RAM Bundle file"); } throw std::ios_base::failure( - folly::to("Error reading RAM Bundle: ", m_bundle.rdstate())); + folly::to("Error reading RAM Bundle: ", m_bundle->rdstate())); } } @@ -92,9 +109,9 @@ void JSIndexedRAMBundle::readBundle( const std::streamsize bytes, const std::ifstream::pos_type position) const { - if (!m_bundle.seekg(position)) { + if (!m_bundle->seekg(position)) { throw std::ios_base::failure( - folly::to("Error reading RAM Bundle: ", m_bundle.rdstate())); + folly::to("Error reading RAM Bundle: ", m_bundle->rdstate())); } readBundle(buffer, bytes); } diff --git a/ReactCommon/cxxreact/JSIndexedRAMBundle.h b/ReactCommon/cxxreact/JSIndexedRAMBundle.h index be6b27f7a84..2f2db3b708f 100644 --- a/ReactCommon/cxxreact/JSIndexedRAMBundle.h +++ b/ReactCommon/cxxreact/JSIndexedRAMBundle.h @@ -5,7 +5,7 @@ #pragma once -#include +#include #include #include @@ -24,6 +24,7 @@ public: // Throws std::runtime_error on failure. JSIndexedRAMBundle(const char *sourceURL); + JSIndexedRAMBundle(std::unique_ptr script); // Throws std::runtime_error on failure. std::unique_ptr getStartupCode(); @@ -51,14 +52,15 @@ private: } }; + void init(); std::string getModuleCode(const uint32_t id) const; void readBundle(char *buffer, const std::streamsize bytes) const; void readBundle( char *buffer, const std::streamsize bytes, - const std::ifstream::pos_type position) const; + const std::istream::pos_type position) const; - mutable std::ifstream m_bundle; + mutable std::unique_ptr m_bundle; ModuleTable m_table; size_t m_baseOffset; std::unique_ptr m_startupCode;