Fixed crash in JSIExecutor::NativeModuleProxy

Summary:
JSIExecutor::NativeModuleProxy is an object created by JSIExecutor and essentially representing that in JavaScript world. Before this change, JSIExecutor::NativeModuleProxy had a raw reference to JSIExecutor which (I believe) caused a crash because JSIExecutor can be deallocated before JSIExecutor::NativeModuleProxy. Now, instead of storing a pointer to JSIExecutor, we store a weak pointer to JSINativeModules which we can safely validate before calling on it.

Changelog: [Internal] Fixed crash in JSIExecutor

Now the configuration looks like this:

```
                                                         + - - - - - - - - - - - - - - - - - - - - -
                                                                        Something else              |
                                                         |     shared_ptr<jsi::Runtime> runtime      --+
                                                                                                    |  |
                                                         + - - - - - - - - - - - - - - - - - - - - -   |
                                                                                                       |
                                                                                                       |
 +------------------------------------------+                                                          |
 |                                          |                                                          |
 |            JSExecutorFactory             |                                                          |        +--------------------------------+-------------------------------+
 |                                          |                                  +-----------------------+        |                                |                               |
 +------------------------------------------+                                  |                                |                                v                               |
                                                                               |                                |          +------------------------------------------+          |
                                                                               +--------------------------+     |          |                                          |          |
                                                                               |                          |     |          |              ModuleRegistry              |          |
                                                                               v                          |     |          |                                          |          |
                                                         +------------------------------------------+     |     |          +------------------------------------------+          |
                                                         |            HermesRuntimeImpl             |     |     |          |                                                     |
                                                         |              (jsi::Runtime)              |--+  |     |          +->+------------------------------------------+       |
                                                         |                                          |  |  |     |          |  |std::unordered_map<std::string, size_t>   |       |
                                                         +------------------------------------------+  |  |     |          |  |modulesByName_                            |       |
                                                                                                       |  |     |          |  |                                          |       |
                                                                                                       |  |     |          |  +------------------------------------------+       |
                                                                                                       |  |     |          +->+------------------------------------------+       |
                                                                               +-----------------------+  |     |             |std::vector<std::unique_ptr<NativeModule>>|       |
                                                                               |                          |     |             |modules_                                  |       |
                                                                               |                          |     |             |                                          |       |
                                                                               v                          |     |             +------------------------------------------+       |
                                                         +------------------------------------------+     |     |                                                                |
                                                         |                                          |     |     |                                                                |
                                                         |      JSIExecutor::NativeModuleProxy      |     |     |                                                                |
                                                         |                                          |     |     |                                                                |
                                                         +------------------------------------------+     |     |                                                                |
+------------------------------------------+             |                                                |     |                                                                |
|                                          |             +->+------------------------------------------+  |     |                                                                |
|             NativeToJsBridge             |                |shared_ptr<JSINativeModules>              |  |     |                                                                |
|                                          |                |nativeModules_                            |  |     |                                                                |
+------------------------------------------+                +------------------------------------------+--+-----+------------------------------------+                           |
|                                                                                                         |     |                                    |                           |
+->+------------------------------------------+                                                           |     |                                    |                           |
|  |unique_ptr<JSExecutor>                    |                                                           |     |                                    |                           |
|  |m_executor                                |                                                           |     |                                    |                           |
|  |(`::destroy()` resets it.)                |                                                           |     |                                    |                           |
|  +------------------------------------------+--------------------------------+                          |     |                                    |                           |
+->+------------------------------------------+                                |                          |     |                                    |                           |
|  |shared_ptr<JsToNativeBridge>              |                                |                          |     |                                    |                           |
|  |m_delegate                                |                                |                          |     |                                    |                           |
|  +------------------------------------------+--+                             v                          |     |                                    |                           |
+->+------------------------------------------+  |       +------------------------------------------+     |     |                                    |                           |
   |shared_ptr<MessageQueueThread>            |  |       |                                          |     |     |                                    |                           |
   |m_executorMessageQueueThread              |  |       | HermesExecutor: JSIExecutor: JSExecutor  |     |     |                                    |                           |
   +------------------------------------------+  |       |                                          |     |     |                                    |                           |
                                                 |       +------------------------------------------+     |     |                                    |                           |
                                                 |       |                                                |     |                                    |                           |
                                                 |       +->+------------------------------------------+  |     |                                    |                           |
                                                 |       |  |shared_ptr<jsi::Runtime>                  |  |     |                                    |                           |
                                                 |       |  |runtime_                                  |  |     |                                    |                           |
                                                 |       |  +------------------------------------------+--+     |                                    |                           |
                                                 |       +->+------------------------------------------+        |                                    |                           |
                                                 |       |  |shared_ptr<JSINativeModules>              |        |                                    |                           |
                                                 |       |  |nativeModules_                            |        |                                    |                           |
                                                 |       |  +------------------------------------------+--------+------------------------------------+                           |
                      +--------------------------+       +->+------------------------------------------+        |                                    |                           |
                      |                                     |std::shared_ptr<ExecutorDelegate>         |        |                                    v                           |
                      |                                     |delegate_                                 |        |              +------------------------------------------+      |
                      |                                     +------------------------------------------+--+     |              |                                          |      |
                      |                                                                                   |     |              |             JSINativeModules             |      |
                      |                                                                                   |     |              |                                          |      |
                      |                                                                                   |     |              +------------------------------------------+      |
                      |                                                                                   |     |              |                                                 |
                      |                                                                                   |     |              +-->+------------------------------------------+  |
                      +-----------------------------------------------------------------------------------+     |                  |m_moduleRegistry                          |  |
                      |                                                                                         |                  |(shared_ptr)                              |  |
                      |                                                                                         |                  +------------------------------------------+--+
                      |                                                                                         |
                      |                                                                                         |
                      v                                                                                         |
+------------------------------------------+                                                                    |
|                                          |                                                                    |
|    JsToNativeBridge: ExecutorDelegate    |                                                                    |
|                                          |                                                                    |
+------------------------------------------+                                                                    |
|                                                                                                               |
+->+------------------------------------------+                                                                 |
   |shared_ptr<ModuleRegistry>                |                                                                 |
   |m_registry                                |                                                                 |
   +------------------------------------------+-----------------------------------------------------------------+

```

Reviewed By: RSNara

Differential Revision: D20817257

fbshipit-source-id: 9ae378dbe880aaabfef7ae783dae2f94ee4b0af5
This commit is contained in:
Valentin Shergin 2020-04-02 11:12:22 -07:00 коммит произвёл Facebook GitHub Bot
Родитель 3d61dc9f36
Коммит b145a82964
2 изменённых файлов: 13 добавлений и 6 удалений

Просмотреть файл

@ -27,14 +27,20 @@ namespace react {
class JSIExecutor::NativeModuleProxy : public jsi::HostObject {
public:
NativeModuleProxy(JSIExecutor &executor) : executor_(executor) {}
NativeModuleProxy(std::shared_ptr<JSINativeModules> nativeModules)
: weakNativeModules_(nativeModules) {}
Value get(Runtime &rt, const PropNameID &name) override {
if (name.utf8(rt) == "name") {
return jsi::String::createFromAscii(rt, "NativeModules");
}
return executor_.nativeModules_.getModule(rt, name);
auto nativeModules = weakNativeModules_.lock();
if (!nativeModules) {
return nullptr;
}
return nativeModules->getModule(rt, name);
}
void set(Runtime &, const PropNameID &, const Value &) override {
@ -43,7 +49,7 @@ class JSIExecutor::NativeModuleProxy : public jsi::HostObject {
}
private:
JSIExecutor &executor_;
std::weak_ptr<JSINativeModules> weakNativeModules_;
};
namespace {
@ -63,7 +69,8 @@ JSIExecutor::JSIExecutor(
RuntimeInstaller runtimeInstaller)
: runtime_(runtime),
delegate_(delegate),
nativeModules_(delegate ? delegate->getModuleRegistry() : nullptr),
nativeModules_(std::make_shared<JSINativeModules>(
delegate ? delegate->getModuleRegistry() : nullptr)),
scopedTimeoutInvoker_(scopedTimeoutInvoker),
runtimeInstaller_(runtimeInstaller) {
runtime_->global().setProperty(
@ -76,7 +83,7 @@ void JSIExecutor::initializeRuntime() {
*runtime_,
"nativeModuleProxy",
Object::createFromHostObject(
*runtime_, std::make_shared<NativeModuleProxy>(*this)));
*runtime_, std::make_shared<NativeModuleProxy>(nativeModules_)));
runtime_->global().setProperty(
*runtime_,

Просмотреть файл

@ -122,7 +122,7 @@ class JSIExecutor : public JSExecutor {
std::shared_ptr<jsi::Runtime> runtime_;
std::shared_ptr<ExecutorDelegate> delegate_;
JSINativeModules nativeModules_;
std::shared_ptr<JSINativeModules> nativeModules_;
std::once_flag bindFlag_;
std::unique_ptr<RAMBundleRegistry> bundleRegistry_;
JSIScopedTimeoutInvoker scopedTimeoutInvoker_;