Skip to content

Commit 84a905a

Browse files
authored
fix: Fix reload crash by also listening to Main JS Runtime in RuntimeAwareCache (#167)
* fix: Fix reload crash by also listening to Main JS Runtime in `RuntimeAwareCache` * Update WKTRuntimeAwareCache.h * Delete WKTRuntimeAwareCache.cpp * Update WKTRuntimeAwareCache.h
1 parent 8565e29 commit 84a905a

File tree

3 files changed

+16
-69
lines changed

3 files changed

+16
-69
lines changed

cpp/WKTJsiWorkletContext.cpp

-3
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,6 @@ void JsiWorkletContext::initialize(
7272
std::function<void(std::function<void()> &&)> jsCallInvoker,
7373
std::function<void(std::function<void()> &&)> workletCallInvoker) {
7474

75-
// Register main runtime
76-
BaseRuntimeAwareCache::setMainJsRuntime(jsRuntime);
77-
7875
_name = name;
7976
_jsRuntime = jsRuntime;
8077
_jsCallInvoker = jsCallInvoker;

cpp/base/WKTRuntimeAwareCache.cpp

-7
This file was deleted.

cpp/base/WKTRuntimeAwareCache.h

+16-59
Original file line numberDiff line numberDiff line change
@@ -12,23 +12,6 @@ namespace RNWorklet {
1212

1313
namespace jsi = facebook::jsi;
1414

15-
class BaseRuntimeAwareCache {
16-
public:
17-
static void setMainJsRuntime(jsi::Runtime *rt) { _mainRuntime = rt; }
18-
19-
protected:
20-
static jsi::Runtime *getMainJsRuntime() {
21-
assert(_mainRuntime != nullptr &&
22-
"Expected main Javascript runtime to be set in the "
23-
"BaseRuntimeAwareCache class.");
24-
25-
return _mainRuntime;
26-
}
27-
28-
private:
29-
static jsi::Runtime *_mainRuntime;
30-
};
31-
3215
/**
3316
* Provides a way to keep data specific to a jsi::Runtime instance that gets
3417
* cleaned up when that runtime is destroyed. This is necessary because JSI does
@@ -38,64 +21,38 @@ class BaseRuntimeAwareCache {
3821
* eventually) will result in a crash (JSI objects keep a pointer to memory
3922
* managed by the runtime, accessing that portion of the memory after runtime is
4023
* deleted is the root cause of that crash).
41-
*
42-
* In order to provide an efficient implementation that does not add an overhead
43-
* for the cases when only a single runtiome is used, which is the primary
44-
* usecase, the following assumption has been made: Only for secondary runtimes
45-
* we track destruction and clean up the store associated with that runtime. For
46-
* the first runtime we assume that the object holding the store is destroyed
47-
* prior to the destruction of that runtime.
48-
*
49-
* The above assumption makes it work without any overhead when only single
50-
* runtime is in use. Specifically, we don't perform any additional operations
51-
* related to tracking runtime lifecycle when only a single runtime is used.
5224
*/
5325
template <typename T>
54-
class RuntimeAwareCache : public BaseRuntimeAwareCache,
55-
public RuntimeLifecycleListener {
26+
class RuntimeAwareCache : public RuntimeLifecycleListener {
5627

5728
public:
5829
void onRuntimeDestroyed(jsi::Runtime *rt) override {
59-
if (getMainJsRuntime() != rt) {
60-
// We are removing a secondary runtime
61-
_secondaryRuntimeCaches.erase(rt);
62-
}
30+
// A runtime has been destroyed, so destroy the related cache.
31+
_runtimeCaches.erase(rt);
6332
}
6433

6534
~RuntimeAwareCache() {
66-
for (auto &cache : _secondaryRuntimeCaches) {
67-
RuntimeLifecycleMonitor::removeListener(
68-
*static_cast<jsi::Runtime *>(cache.first), this);
35+
for (auto &cache : _runtimeCaches) {
36+
// remove all `onRuntimeDestroyed` listeners.
37+
RuntimeLifecycleMonitor::removeListener(*cache.first, this);
6938
}
7039
}
7140

7241
T &get(jsi::Runtime &rt) {
73-
// We check if we're accessing the main runtime - this is the happy path
74-
// to avoid us having to lookup by runtime for caches that only has a single
75-
// runtime
76-
if (getMainJsRuntime() == &rt) {
77-
return _primaryCache;
78-
} else {
79-
if (_secondaryRuntimeCaches.count(&rt) == 0) {
80-
// we only add listener when the secondary runtime is used, this assumes
81-
// that the secondary runtime is terminated first. This lets us avoid
82-
// additional complexity for the majority of cases when objects are not
83-
// shared between runtimes. Otherwise we'd have to register all objects
84-
// with the RuntimeMonitor as opposed to only registering ones that are
85-
// used in secondary runtime. Note that we can't register listener here
86-
// with the primary runtime as it may run on a separate thread.
87-
RuntimeLifecycleMonitor::addListener(rt, this);
88-
89-
T cache;
90-
_secondaryRuntimeCaches.emplace(&rt, std::move(cache));
91-
}
42+
if (_runtimeCaches.count(&rt) == 0) {
43+
// This is the first time this Runtime has been accessed.
44+
// We set up a `onRuntimeDestroyed` listener for it and
45+
// initialize the cache map.
46+
RuntimeLifecycleMonitor::addListener(rt, this);
47+
48+
T cache;
49+
_runtimeCaches.emplace(&rt, std::move(cache));
9250
}
93-
return _secondaryRuntimeCaches.at(&rt);
51+
return _runtimeCaches.at(&rt);
9452
}
9553

9654
private:
97-
std::unordered_map<void *, T> _secondaryRuntimeCaches;
98-
T _primaryCache;
55+
std::unordered_map<jsi::Runtime *, T> _runtimeCaches;
9956
};
10057

10158
} // namespace RNWorklet

0 commit comments

Comments
 (0)