Skip to content

Commit 6fc681d

Browse files
committed
Fix ObjC callback crash
1 parent 1742e42 commit 6fc681d

24 files changed

Lines changed: 1766 additions & 62 deletions

NativeScript/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,7 @@ if(ENABLE_JS_RUNTIME)
153153
runtime/modules/module/ModuleInternal.cpp
154154
runtime/modules/node/Node.cpp
155155
runtime/modules/node/FS.cpp
156+
runtime/modules/node/Path.cpp
156157
runtime/modules/performance/Performance.cpp
157158
runtime/Bundle.mm
158159
runtime/modules/timers/Timers.mm

NativeScript/ffi/Block.mm

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
#import <Foundation/Foundation.h>
33
#include <cstdint>
44
#include <cstring>
5+
#include <mutex>
56
#include <unordered_map>
67
#include "Interop.h"
78
#include "ObjCBridge.h"
@@ -37,11 +38,15 @@
3738
constexpr int kBlockRefCountOne = (1 << 1);
3839
constexpr int kBlockHasSignature = (1 << 30);
3940
std::unordered_map<void*, napi_ref> g_blockToJsFunction;
41+
std::mutex g_blockToJsFunctionMutex;
4042

4143
void block_copy(void* dest, void* src) {
4244
auto dst = static_cast<Block_literal_1*>(dest);
4345
auto source = static_cast<Block_literal_1*>(src);
4446
dst->closure = source->closure;
47+
if (dst->closure != nullptr) {
48+
dst->closure->retain();
49+
}
4550
}
4651

4752
void block_release(void* src) {
@@ -51,28 +56,32 @@ void block_release(void* src) {
5156
}
5257

5358
if (block->closure != nullptr && block->closure->env != nullptr) {
59+
std::lock_guard<std::mutex> lock(g_blockToJsFunctionMutex);
5460
auto it = g_blockToJsFunction.find(block);
5561
if (it != g_blockToJsFunction.end()) {
56-
napi_delete_reference(block->closure->env, it->second);
62+
if (std::this_thread::get_id() == block->closure->jsThreadId) {
63+
napi_delete_reference(block->closure->env, it->second);
64+
}
5765
g_blockToJsFunction.erase(it);
5866
}
5967
}
6068

6169
if (block->closure != nullptr) {
62-
delete block->closure;
63-
block->closure = nullptr;
70+
block->closure->release();
6471
}
72+
block->closure = nullptr;
6573
}
6674

6775
Block_descriptor_1 kBlockDescriptor = {
6876
.reserved = 0,
6977
.size = sizeof(Block_literal_1),
7078
.copy_helper = block_copy,
7179
.dispose_helper = block_release,
72-
.signature = nullptr,
80+
.signature = "v@?",
7381
};
7482

7583
inline napi_value getCachedBlockJsFunction(napi_env env, void* blockPtr) {
84+
std::lock_guard<std::mutex> lock(g_blockToJsFunctionMutex);
7685
auto it = g_blockToJsFunction.find(blockPtr);
7786
if (it == g_blockToJsFunction.end()) {
7887
return nullptr;
@@ -89,6 +98,7 @@ inline void cacheBlockJsFunction(napi_env env, void* blockPtr, napi_value jsFunc
8998
if (blockPtr == nullptr || jsFunction == nullptr) {
9099
return;
91100
}
101+
std::lock_guard<std::mutex> lock(g_blockToJsFunctionMutex);
92102
if (g_blockToJsFunction.find(blockPtr) != g_blockToJsFunction.end()) {
93103
return;
94104
}
@@ -104,16 +114,19 @@ void block_finalize(napi_env env, void* data, void* hint) {
104114
return;
105115
}
106116

107-
auto it = g_blockToJsFunction.find(block);
108-
if (it != g_blockToJsFunction.end()) {
109-
napi_delete_reference(env, it->second);
110-
g_blockToJsFunction.erase(it);
117+
{
118+
std::lock_guard<std::mutex> lock(g_blockToJsFunctionMutex);
119+
auto it = g_blockToJsFunction.find(block);
120+
if (it != g_blockToJsFunction.end()) {
121+
napi_delete_reference(env, it->second);
122+
g_blockToJsFunction.erase(it);
123+
}
111124
}
112125

113126
if (block->closure != nullptr) {
114-
delete block->closure;
115-
block->closure = nullptr;
127+
block->closure->release();
116128
}
129+
block->closure = nullptr;
117130

118131
free(block);
119132
}
@@ -136,7 +149,7 @@ id registerBlock(napi_env env, Closure* closure, napi_value callback) {
136149
}
137150

138151
block->isa = mallocBlockISA != nullptr ? mallocBlockISA : stackBlockISA;
139-
block->flags = kBlockNeedsFree | kBlockHasCopyDispose | kBlockRefCountOne;
152+
block->flags = kBlockNeedsFree | kBlockHasCopyDispose | kBlockRefCountOne | kBlockHasSignature;
140153
block->reserved = 0;
141154
block->invoke = closure->fnptr;
142155
block->descriptor = &kBlockDescriptor;

NativeScript/ffi/ClassMember.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,8 @@ class ObjCClassMember {
7676
: bridgeState(bridgeState),
7777
methodOrGetter(MethodDescriptor(selector, offset)),
7878
returnOwned((flags & metagen::mdMemberReturnOwned) != 0),
79-
classMethod((flags & metagen::mdMemberStatic) != 0) {}
79+
classMethod((flags & metagen::mdMemberStatic) != 0),
80+
cls(nullptr) {}
8081

8182
ObjCClassMember(ObjCBridgeState* bridgeState, SEL getterSelector,
8283
SEL setterSelector, MDSectionOffset getterOffset,
@@ -85,7 +86,8 @@ class ObjCClassMember {
8586
methodOrGetter(MethodDescriptor(getterSelector, getterOffset)),
8687
setter(MethodDescriptor(setterSelector, setterOffset)),
8788
returnOwned((flags & metagen::mdMemberReturnOwned) != 0),
88-
classMethod((flags & metagen::mdMemberStatic) != 0) {
89+
classMethod((flags & metagen::mdMemberStatic) != 0),
90+
cls(nullptr) {
8991
methodOrGetter.isProperty = true;
9092
setter.isProperty = true;
9193
}

0 commit comments

Comments
 (0)