From bdb084e8a8ce6af3f783bb717ca5dfe219f0b6a0 Mon Sep 17 00:00:00 2001 From: Marc Horowitz Date: Tue, 4 Dec 2018 11:42:24 -0800 Subject: [PATCH] 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 --- ReactCommon/cxxreact/Instance.cpp | 2 ++ ReactCommon/jsi/JSCRuntime.cpp | 40 ++++++++++++++++++++++++------- 2 files changed, 33 insertions(+), 9 deletions(-) diff --git a/ReactCommon/cxxreact/Instance.cpp b/ReactCommon/cxxreact/Instance.cpp index a49f1d60d9f552..aa434764cc9a00 100644 --- a/ReactCommon/cxxreact/Instance.cpp +++ b/ReactCommon/cxxreact/Instance.cpp @@ -53,6 +53,8 @@ void Instance::initializeBridge( m_syncCV.notify_all(); }); + // If the NativeToJsBridge ctor throws an exception, this check will + // likely happen before before the redbox can be rendered. CHECK(nativeToJsBridge_); } diff --git a/ReactCommon/jsi/JSCRuntime.cpp b/ReactCommon/jsi/JSCRuntime.cpp index 3e9543653a63ea..53e0a29f3e4956 100644 --- a/ReactCommon/jsi/JSCRuntime.cpp +++ b/ReactCommon/jsi/JSCRuntime.cpp @@ -28,6 +28,14 @@ struct Lock { void unlock(const jsc::JSCRuntime&) const {} }; +#if __has_builtin(__builtin_expect) +#define JSC_LIKELY(EXPR) __builtin_expect((bool)(EXPR), true) +#define JSC_UNLIKELY(EXPR) __builtin_expect((bool)(EXPR), false) +#else +#define JSC_LIKELY(EXPR) (EXPR) +#define JSC_UNLIKELY(EXPR) (EXPR) +#endif + class JSCRuntime : public jsi::Runtime { public: // Creates new context in new context group @@ -191,23 +199,32 @@ class JSCRuntime : public jsi::Runtime { void checkException(JSValueRef exc, const char* msg); void checkException(JSValueRef res, JSValueRef exc, const char* msg); + void checkThreadId() { +#ifndef NDEBUG + if (JSC_UNLIKELY(tid_ != std::this_thread::get_id())) { + // 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. This could be called anywhere, but for + // now, it's called only in a few less frequently used places to + // avoid unnecessary checks. + throw std::logic_error("Detected JSC thread hazard"); + } +#endif + } + JSGlobalContextRef ctx_; std::atomic ctxInvalid_; std::string desc_; #ifndef NDEBUG mutable std::atomic objectCounter_; mutable std::atomic stringCounter_; + std::thread::id tid_; #endif }; -#if __has_builtin(__builtin_expect) -#define JSC_LIKELY(EXPR) __builtin_expect((bool)(EXPR), true) -#define JSC_UNLIKELY(EXPR) __builtin_expect((bool)(EXPR), false) -#else -#define JSC_LIKELY(EXPR) (EXPR) -#define JSC_UNLIKELY(EXPR) (EXPR) -#endif - #define JSC_ASSERT(x) \ do { \ if (JSC_UNLIKELY(!!(x))) { \ @@ -292,7 +309,8 @@ JSCRuntime::JSCRuntime(JSGlobalContextRef ctx) #ifndef NDEBUG , objectCounter_(0), - stringCounter_(0) + stringCounter_(0), + tid_(std::this_thread::get_id()) #endif { } @@ -317,6 +335,8 @@ JSCRuntime::~JSCRuntime() { void JSCRuntime::evaluateJavaScript( std::unique_ptr buffer, const std::string& sourceURL) { + checkThreadId(); + std::string tmp( reinterpret_cast(buffer->data()), buffer->size()); JSStringRef sourceRef = JSStringCreateWithUTF8CString(tmp.c_str()); @@ -335,6 +355,8 @@ void JSCRuntime::evaluateJavaScript( } jsi::Object JSCRuntime::global() { + checkThreadId(); + return createObject(JSContextGetGlobalObject(ctx_)); }