Skip to content

Commit bdb084e

Browse files
mhorowitzfacebook-github-bot
authored andcommitted
Check for thread consistency in JSCRuntime
Summary: In the version of JSC on iOS 11, creating a JSContext on one thread and using it on another can trigger subtle and nearly impossible to debug reentrancy-related crashes in the VM (see https://bugs.webkit.org/show_bug.cgi?id=186827). In !NDEBUG builds, check for this case and throw an exception, so it can be detected early. Reviewed By: amnn Differential Revision: D13313264 fbshipit-source-id: ee85435c20e23c8520495ce743d2f91f2eeada5c
1 parent 512676c commit bdb084e

File tree

2 files changed

+33
-9
lines changed

2 files changed

+33
-9
lines changed

ReactCommon/cxxreact/Instance.cpp

+2
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,8 @@ void Instance::initializeBridge(
5353
m_syncCV.notify_all();
5454
});
5555

56+
// If the NativeToJsBridge ctor throws an exception, this check will
57+
// likely happen before before the redbox can be rendered.
5658
CHECK(nativeToJsBridge_);
5759
}
5860

ReactCommon/jsi/JSCRuntime.cpp

+31-9
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,14 @@ struct Lock {
2828
void unlock(const jsc::JSCRuntime&) const {}
2929
};
3030

31+
#if __has_builtin(__builtin_expect)
32+
#define JSC_LIKELY(EXPR) __builtin_expect((bool)(EXPR), true)
33+
#define JSC_UNLIKELY(EXPR) __builtin_expect((bool)(EXPR), false)
34+
#else
35+
#define JSC_LIKELY(EXPR) (EXPR)
36+
#define JSC_UNLIKELY(EXPR) (EXPR)
37+
#endif
38+
3139
class JSCRuntime : public jsi::Runtime {
3240
public:
3341
// Creates new context in new context group
@@ -191,23 +199,32 @@ class JSCRuntime : public jsi::Runtime {
191199
void checkException(JSValueRef exc, const char* msg);
192200
void checkException(JSValueRef res, JSValueRef exc, const char* msg);
193201

202+
void checkThreadId() {
203+
#ifndef NDEBUG
204+
if (JSC_UNLIKELY(tid_ != std::this_thread::get_id())) {
205+
// In the version of JSC on iOS 11, creating a JSContext on one
206+
// thread and using it on another can trigger subtle and nearly
207+
// impossible to debug reentrancy-related crashes in the VM (see
208+
// https://bugs.webkit.org/show_bug.cgi?id=186827). In !NDEBUG
209+
// builds, check for this case and throw an exception, so it can
210+
// be detected early. This could be called anywhere, but for
211+
// now, it's called only in a few less frequently used places to
212+
// avoid unnecessary checks.
213+
throw std::logic_error("Detected JSC thread hazard");
214+
}
215+
#endif
216+
}
217+
194218
JSGlobalContextRef ctx_;
195219
std::atomic<bool> ctxInvalid_;
196220
std::string desc_;
197221
#ifndef NDEBUG
198222
mutable std::atomic<intptr_t> objectCounter_;
199223
mutable std::atomic<intptr_t> stringCounter_;
224+
std::thread::id tid_;
200225
#endif
201226
};
202227

203-
#if __has_builtin(__builtin_expect)
204-
#define JSC_LIKELY(EXPR) __builtin_expect((bool)(EXPR), true)
205-
#define JSC_UNLIKELY(EXPR) __builtin_expect((bool)(EXPR), false)
206-
#else
207-
#define JSC_LIKELY(EXPR) (EXPR)
208-
#define JSC_UNLIKELY(EXPR) (EXPR)
209-
#endif
210-
211228
#define JSC_ASSERT(x) \
212229
do { \
213230
if (JSC_UNLIKELY(!!(x))) { \
@@ -292,7 +309,8 @@ JSCRuntime::JSCRuntime(JSGlobalContextRef ctx)
292309
#ifndef NDEBUG
293310
,
294311
objectCounter_(0),
295-
stringCounter_(0)
312+
stringCounter_(0),
313+
tid_(std::this_thread::get_id())
296314
#endif
297315
{
298316
}
@@ -317,6 +335,8 @@ JSCRuntime::~JSCRuntime() {
317335
void JSCRuntime::evaluateJavaScript(
318336
std::unique_ptr<const jsi::Buffer> buffer,
319337
const std::string& sourceURL) {
338+
checkThreadId();
339+
320340
std::string tmp(
321341
reinterpret_cast<const char*>(buffer->data()), buffer->size());
322342
JSStringRef sourceRef = JSStringCreateWithUTF8CString(tmp.c_str());
@@ -335,6 +355,8 @@ void JSCRuntime::evaluateJavaScript(
335355
}
336356

337357
jsi::Object JSCRuntime::global() {
358+
checkThreadId();
359+
338360
return createObject(JSContextGetGlobalObject(ctx_));
339361
}
340362

0 commit comments

Comments
 (0)