Skip to content

Commit ced2953

Browse files
committed
fix(Worklets): time-of-check and time-of-use destructor safety (#9790)
## Summary Currently the way we check if the runtime is alive for safe destructor invocations has a chance to lead to crashes anyway, since the runtime can get cleaned up immediately after the check, since it lacks a locking mechanism. ## Test plan The test example from #9789 works. (cherry picked from commit 71774cb)
1 parent 7dc7ff6 commit ced2953

5 files changed

Lines changed: 40 additions & 22 deletions

File tree

packages/react-native-worklets/Common/cpp/worklets/Registries/WorkletRuntimeRegistry.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,6 @@
55
namespace worklets {
66

77
std::set<jsi::Runtime *> WorkletRuntimeRegistry::registry_{};
8-
std::mutex WorkletRuntimeRegistry::mutex_{};
8+
std::shared_mutex WorkletRuntimeRegistry::mutex_{};
99

1010
} // namespace worklets

packages/react-native-worklets/Common/cpp/worklets/Registries/WorkletRuntimeRegistry.h

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,9 @@
33
#include <jsi/jsi.h>
44
#include <react/debug/react_native_assert.h>
55

6-
#include <mutex>
76
#include <set>
7+
#include <shared_mutex>
8+
#include <utility>
89

910
using namespace facebook;
1011

@@ -13,27 +14,29 @@ namespace worklets {
1314
class WorkletRuntimeRegistry {
1415
private:
1516
static std::set<jsi::Runtime *> registry_;
16-
static std::mutex mutex_; // Protects `registry_`.
17+
static std::shared_mutex mutex_;
1718

18-
WorkletRuntimeRegistry() {} // private ctor
19+
WorkletRuntimeRegistry() {}
1920

2021
static void registerRuntime(jsi::Runtime &runtime) {
21-
std::lock_guard<std::mutex> lock(mutex_);
22+
std::lock_guard<std::shared_mutex> lock(mutex_);
2223
registry_.insert(&runtime);
2324
}
2425

2526
static void unregisterRuntime(jsi::Runtime &runtime) {
26-
std::lock_guard<std::mutex> lock(mutex_);
27+
std::lock_guard<std::shared_mutex> lock(mutex_);
2728
registry_.erase(&runtime);
2829
}
2930

3031
friend class WorkletRuntimeCollector;
3132

3233
public:
33-
static bool isRuntimeAlive(jsi::Runtime *runtime) {
34+
template <typename TFn>
35+
static void runWhileLocked(jsi::Runtime *runtime, TFn &&fn) {
3436
react_native_assert(runtime != nullptr && "runtime is nullptr");
35-
std::lock_guard<std::mutex> lock(mutex_);
36-
return registry_.find(runtime) != registry_.end();
37+
std::shared_lock<std::shared_mutex> lock(mutex_);
38+
const bool isAlive = registry_.find(runtime) != registry_.end();
39+
std::forward<TFn>(fn)(isAlive);
3740
}
3841
};
3942

packages/react-native-worklets/Common/cpp/worklets/SharedItems/Serializable.h

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,16 @@ inline void freeWithoutCallingDestructor(std::unique_ptr<jsi::Value> &value) {
3030
}
3131

3232
inline void cleanupRuntimeAware(jsi::Runtime *rt, std::unique_ptr<jsi::Value> &value) {
33-
if (rt != nullptr && !WorkletRuntimeRegistry::isRuntimeAlive(rt)) {
34-
freeWithoutCallingDestructor(value);
33+
if (value == nullptr || rt == nullptr) {
34+
return;
3535
}
36+
WorkletRuntimeRegistry::runWhileLocked(rt, [&value](bool isAlive) {
37+
if (isAlive) {
38+
value.reset();
39+
} else {
40+
freeWithoutCallingDestructor(value);
41+
}
42+
});
3643
}
3744

3845
template <typename BaseClass>

packages/react-native-worklets/Common/cpp/worklets/SharedItems/SerializableRemoteFunction.cpp

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,13 @@ jsi::Function getRemoteFunctionUnpacker(jsi::Runtime &rt) {
2323

2424
SerializableRemoteFunction::~SerializableRemoteFunction() {
2525
if (isHostedOnRNRuntime()) {
26-
if (rnRuntimeStatus_->isDead()) {
27-
freeWithoutCallingDestructor(function_);
28-
} else {
29-
function_.reset();
30-
}
26+
rnRuntimeStatus_->runWhileLocked([this](bool isDead) {
27+
if (isDead) {
28+
freeWithoutCallingDestructor(function_);
29+
} else {
30+
function_.reset();
31+
}
32+
});
3133
} else {
3234
cleanupRuntimeAware(hostRuntime_, function_);
3335
}
Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,27 @@
11
#pragma once
22

3-
#include <atomic>
3+
#include <shared_mutex>
4+
#include <utility>
45

56
namespace worklets {
67

78
class RNRuntimeStatus {
89
public:
9-
[[nodiscard]] bool isDead() const {
10-
return isDead_;
11-
}
12-
1310
void setDead() {
11+
std::lock_guard<std::shared_mutex> lock(mutex_);
1412
isDead_ = true;
1513
}
1614

15+
template <typename TFn>
16+
void runWhileLocked(TFn &&fn) {
17+
std::shared_lock<std::shared_mutex> lock(mutex_);
18+
const bool isDead = isDead_;
19+
std::forward<TFn>(fn)(isDead);
20+
}
21+
1722
private:
18-
std::atomic_bool isDead_ = false;
23+
bool isDead_ = false;
24+
std::shared_mutex mutex_;
1925
};
2026

2127
} // namespace worklets

0 commit comments

Comments
 (0)