Switch to synchronous strategy for unprotect
Summary: Older versions of JSC (ios 11 and before) have a bug which I believe the ProtectionQueue mechanism tickles: https://bugs.webkit.org/show_bug.cgi?id=186827 This removes the ProtectionQueue and replaces it with an atomic flag to avoid calling unprotect after VM shutdown. This also fixes a race condition in shutdown. Reviewed By: danzimm Differential Revision: D12969953 fbshipit-source-id: fa3a14f3207be67a987ac3cf0fc1c9ce88837b0b
This commit is contained in:
Родитель
2b01da073c
Коммит
bf2500e38e
|
@ -6,6 +6,7 @@
|
|||
#include "JSCRuntime.h"
|
||||
|
||||
#include <JavaScriptCore/JavaScript.h>
|
||||
#include <atomic>
|
||||
#include <condition_variable>
|
||||
#include <cstdlib>
|
||||
#include <mutex>
|
||||
|
@ -13,16 +14,11 @@
|
|||
#include <sstream>
|
||||
#include <thread>
|
||||
|
||||
#ifndef NDEBUG
|
||||
#include <atomic>
|
||||
#endif
|
||||
|
||||
namespace facebook {
|
||||
namespace jsc {
|
||||
|
||||
namespace detail {
|
||||
class ArgsConverter;
|
||||
class ProtectionQueue;
|
||||
} // namespace detail
|
||||
|
||||
class JSCRuntime;
|
||||
|
@ -65,7 +61,6 @@ class JSCRuntime : public jsi::Runtime {
|
|||
|
||||
protected:
|
||||
friend class detail::ArgsConverter;
|
||||
friend class detail::ProtectionQueue;
|
||||
class JSCStringValue final : public PointerValue {
|
||||
#ifndef NDEBUG
|
||||
JSCStringValue(JSStringRef str, std::atomic<intptr_t>& counter);
|
||||
|
@ -83,31 +78,26 @@ class JSCRuntime : public jsi::Runtime {
|
|||
};
|
||||
|
||||
class JSCObjectValue final : public PointerValue {
|
||||
#ifndef NDEBUG
|
||||
JSCObjectValue(
|
||||
JSGlobalContextRef ctx,
|
||||
detail::ProtectionQueue& pq,
|
||||
JSObjectRef obj,
|
||||
std::atomic<intptr_t>& counter);
|
||||
#else
|
||||
JSCObjectValue(
|
||||
JSGlobalContextRef context,
|
||||
detail::ProtectionQueue& pq,
|
||||
JSObjectRef obj);
|
||||
const std::atomic<bool>& ctxInvalid,
|
||||
JSObjectRef obj
|
||||
#ifndef NDEBUG
|
||||
,
|
||||
std::atomic<intptr_t>& counter
|
||||
#endif
|
||||
);
|
||||
|
||||
void invalidate() override;
|
||||
void unprotect();
|
||||
|
||||
JSGlobalContextRef ctx_;
|
||||
const std::atomic<bool>& ctxInvalid_;
|
||||
JSObjectRef obj_;
|
||||
detail::ProtectionQueue& protectionQueue_;
|
||||
#ifndef NDEBUG
|
||||
std::atomic<intptr_t>& counter_;
|
||||
#endif
|
||||
protected:
|
||||
friend class JSCRuntime;
|
||||
friend class detail::ProtectionQueue;
|
||||
};
|
||||
|
||||
PointerValue* cloneString(const Runtime::PointerValue* pv) override;
|
||||
|
@ -202,10 +192,8 @@ class JSCRuntime : public jsi::Runtime {
|
|||
void checkException(JSValueRef res, JSValueRef exc, const char* msg);
|
||||
|
||||
JSGlobalContextRef ctx_;
|
||||
std::atomic<bool> ctxInvalid_;
|
||||
std::string desc_;
|
||||
// We make this a pointer so that we can control explicitly when it's deleted
|
||||
// namely before the context is released.
|
||||
mutable std::unique_ptr<detail::ProtectionQueue> protectionQueue_;
|
||||
#ifndef NDEBUG
|
||||
mutable std::atomic<intptr_t> objectCounter_;
|
||||
mutable std::atomic<intptr_t> stringCounter_;
|
||||
|
@ -293,98 +281,6 @@ std::string to_string(void* value) {
|
|||
}
|
||||
} // namespace
|
||||
|
||||
// UnprotectQueue
|
||||
namespace detail {
|
||||
class ProtectionQueue {
|
||||
public:
|
||||
ProtectionQueue()
|
||||
: ctxInvalid_(false)
|
||||
, shuttingDown_(false)
|
||||
#ifndef NDEBUG
|
||||
,
|
||||
didShutdown_ {
|
||||
false
|
||||
}
|
||||
#endif
|
||||
, unprotectorThread_(&ProtectionQueue::unprotectThread, this) {}
|
||||
|
||||
void setContextInvalid() {
|
||||
std::lock_guard<std::mutex> locker(mutex_);
|
||||
ctxInvalid_ = true;
|
||||
}
|
||||
|
||||
void shutdown() {
|
||||
{
|
||||
std::lock_guard<std::mutex> locker(mutex_);
|
||||
assert(ctxInvalid_);
|
||||
shuttingDown_ = true;
|
||||
notEmpty_.notify_one();
|
||||
}
|
||||
unprotectorThread_.join();
|
||||
}
|
||||
|
||||
void push(JSCRuntime::JSCObjectValue* value) {
|
||||
std::lock_guard<std::mutex> locker(mutex_);
|
||||
assert(!didShutdown_);
|
||||
queue_.push(value);
|
||||
notEmpty_.notify_one();
|
||||
}
|
||||
|
||||
private:
|
||||
// This this the function that runs in the background deleting (and thus
|
||||
// unprotecting JSObjectRefs as need be). This needs to be explicitly on a
|
||||
// separate thread so that we don't have the API lock when `JSValueUnprotect`
|
||||
// is called already (i.e. if we did this on the same thread that calls
|
||||
// invalidate() on an Object then we might be in the middle of a GC pass, and
|
||||
// already have the API lock).
|
||||
void unprotectThread() {
|
||||
#if defined(__APPLE__)
|
||||
pthread_setname_np("jsc-protectionqueue-unprotectthread");
|
||||
#endif
|
||||
|
||||
std::unique_lock<std::mutex> locker(mutex_);
|
||||
while (!shuttingDown_ || !queue_.empty()) {
|
||||
if (queue_.empty()) {
|
||||
// This will wake up when shuttingDown_ becomes true
|
||||
notEmpty_.wait(locker);
|
||||
} else {
|
||||
JSCRuntime::JSCObjectValue* value = queue_.front();
|
||||
queue_.pop();
|
||||
// We need to drop the lock here since this calls JSValueUnprotect and
|
||||
// that may make another GC pass, which could call another finalizer
|
||||
// and thus attempt to push to this queue then, and deadlock.
|
||||
locker.unlock();
|
||||
if (ctxInvalid_) {
|
||||
value->ctx_ = nullptr;
|
||||
}
|
||||
value->unprotect();
|
||||
locker.lock();
|
||||
}
|
||||
}
|
||||
#ifndef NDEBUG
|
||||
didShutdown_ = true;
|
||||
#endif
|
||||
}
|
||||
// Used to lock the queue_/shuttingDown_ ivars
|
||||
std::mutex mutex_;
|
||||
// Used to signal queue_ empty status changing
|
||||
std::condition_variable notEmpty_;
|
||||
// The actual underlying queue
|
||||
std::queue<JSCRuntime::JSCObjectValue*> queue_;
|
||||
// A flag which is set before shutting down JSC
|
||||
bool ctxInvalid_;
|
||||
// A flag dictating whether or not we need to stop all execution
|
||||
bool shuttingDown_;
|
||||
#ifndef NDEBUG
|
||||
bool didShutdown_;
|
||||
#endif
|
||||
// The thread that dequeues and processes the queue. Note this is the last
|
||||
// member on purpose so the thread starts up after all state has been
|
||||
// properly initialized
|
||||
std::thread unprotectorThread_;
|
||||
};
|
||||
} // namespace detail
|
||||
|
||||
JSCRuntime::JSCRuntime()
|
||||
: JSCRuntime(JSGlobalContextCreateInGroup(nullptr, nullptr)) {
|
||||
JSGlobalContextRelease(ctx_);
|
||||
|
@ -392,7 +288,7 @@ JSCRuntime::JSCRuntime()
|
|||
|
||||
JSCRuntime::JSCRuntime(JSGlobalContextRef ctx)
|
||||
: ctx_(JSGlobalContextRetain(ctx)),
|
||||
protectionQueue_(std::make_unique<detail::ProtectionQueue>())
|
||||
ctxInvalid_(false)
|
||||
#ifndef NDEBUG
|
||||
,
|
||||
objectCounter_(0),
|
||||
|
@ -405,15 +301,11 @@ JSCRuntime::~JSCRuntime() {
|
|||
// On shutting down and cleaning up: when JSC is actually torn down,
|
||||
// it calls JSC::Heap::lastChanceToFinalize internally which
|
||||
// finalizes anything left over. But at this point,
|
||||
// JSUnprotectValue() can no longer be called. So there's a
|
||||
// multiphase shutdown here. We tell the protection queue that the
|
||||
// VM is invalid, which causes it not to call JSUnprotectValue. but
|
||||
// it will decrement its counters, if !NDEBUG. Then we shut down
|
||||
// the VM, which will clean everything up. Finally, we shut down
|
||||
// the queue itself.
|
||||
protectionQueue_->setContextInvalid();
|
||||
// JSValueUnprotect() can no longer be called. We use an
|
||||
// atomic<bool> to avoid unsafe unprotects happening after shutdown
|
||||
// has started.
|
||||
ctxInvalid_ = true;
|
||||
JSGlobalContextRelease(ctx_);
|
||||
protectionQueue_->shutdown();
|
||||
#ifndef NDEBUG
|
||||
assert(
|
||||
objectCounter_ == 0 && "JSCRuntime destroyed with a dangling API object");
|
||||
|
@ -473,15 +365,9 @@ JSCRuntime::JSCStringValue::JSCStringValue(JSStringRef str)
|
|||
#endif
|
||||
|
||||
void JSCRuntime::JSCStringValue::invalidate() {
|
||||
// We want to immediately JSStringRelease once a String is released,
|
||||
// and queue a JSObjectRef to unprotected (see comment on
|
||||
// ProtectionQueue::unprotectThread above).
|
||||
//
|
||||
// These JSC{String,Object}Value objects are implicitly owned by the
|
||||
// {String,Object} objects, thus when a String/Object is destructed
|
||||
// the JSC{String,Object}Value should be released (again this has
|
||||
// the caveat that objects must be unprotected on a separate
|
||||
// thread).
|
||||
// the JSC{String,Object}Value should be released.
|
||||
#ifndef NDEBUG
|
||||
counter_ -= 1;
|
||||
#endif
|
||||
|
@ -492,7 +378,7 @@ void JSCRuntime::JSCStringValue::invalidate() {
|
|||
|
||||
JSCRuntime::JSCObjectValue::JSCObjectValue(
|
||||
JSGlobalContextRef ctx,
|
||||
detail::ProtectionQueue& pq,
|
||||
const std::atomic<bool>& ctxInvalid,
|
||||
JSObjectRef obj
|
||||
#ifndef NDEBUG
|
||||
,
|
||||
|
@ -500,8 +386,8 @@ JSCRuntime::JSCObjectValue::JSCObjectValue(
|
|||
#endif
|
||||
)
|
||||
: ctx_(ctx),
|
||||
obj_(obj),
|
||||
protectionQueue_(pq)
|
||||
ctxInvalid_(ctxInvalid),
|
||||
obj_(obj)
|
||||
#ifndef NDEBUG
|
||||
,
|
||||
counter_(counter)
|
||||
|
@ -514,16 +400,32 @@ JSCRuntime::JSCObjectValue::JSCObjectValue(
|
|||
}
|
||||
|
||||
void JSCRuntime::JSCObjectValue::invalidate() {
|
||||
// See comment in JSCRuntime::JSCStringValue::invalidate as well as
|
||||
// on ProtectionQueue::unprotectThread.
|
||||
protectionQueue_.push(this);
|
||||
}
|
||||
|
||||
void JSCRuntime::JSCObjectValue::unprotect() {
|
||||
#ifndef NDEBUG
|
||||
counter_ -= 1;
|
||||
#endif
|
||||
if (ctx_) {
|
||||
// When shutting down the VM, if there is a HostObject which
|
||||
// contains or otherwise owns a jsi::Object, then the final GC will
|
||||
// finalize the HostObject, leading to a call to invalidate(). But
|
||||
// at that point, making calls to JSValueUnprotect will crash.
|
||||
// It is up to the application to make sure that any other calls to
|
||||
// invalidate() happen before VM destruction; see the comment on
|
||||
// jsi::Runtime.
|
||||
//
|
||||
// Another potential concern here is that in the non-shutdown case,
|
||||
// if a HostObject is GCd, JSValueUnprotect will be called from the
|
||||
// JSC finalizer. The documentation warns against this: "You must
|
||||
// not call any function that may cause a garbage collection or an
|
||||
// allocation of a garbage collected object from within a
|
||||
// JSObjectFinalizeCallback. This includes all functions that have a
|
||||
// JSContextRef parameter." However, an audit of the source code for
|
||||
// JSValueUnprotect in late 2018 shows that it cannot cause
|
||||
// allocation or a GC, and further, this code has not changed in
|
||||
// about two years. In the future, we may choose to reintroduce the
|
||||
// mechanism previously used here which uses a separate thread for
|
||||
// JSValueUnprotect, in order to conform to the documented API, but
|
||||
// use the "unsafe" synchronous version on iOS 11 and earlier.
|
||||
|
||||
if (!ctxInvalid_) {
|
||||
JSValueUnprotect(ctx_, obj_);
|
||||
}
|
||||
delete this;
|
||||
|
@ -1222,9 +1124,9 @@ jsi::Runtime::PointerValue* JSCRuntime::makeObjectValue(
|
|||
objectRef = JSObjectMake(ctx_, nullptr, nullptr);
|
||||
}
|
||||
#ifndef NDEBUG
|
||||
return new JSCObjectValue(ctx_, *protectionQueue_, objectRef, objectCounter_);
|
||||
return new JSCObjectValue(ctx_, ctxInvalid_, objectRef, objectCounter_);
|
||||
#else
|
||||
return new JSCObjectValue(ctx_, *protectionQueue_, objectRef);
|
||||
return new JSCObjectValue(ctx_, ctxInvalid_, objectRef);
|
||||
#endif
|
||||
}
|
||||
|
||||
|
|
|
@ -109,7 +109,7 @@ class JSI_EXPORT HostObject {
|
|||
};
|
||||
|
||||
/// Represents a JS runtime. Movable, but not copyable. Note that
|
||||
/// this object is not thread-aware, but cannot be used safely from
|
||||
/// this object may not be thread-aware, but cannot be used safely from
|
||||
/// multiple threads at once. The application is responsible for
|
||||
/// ensuring that it is used safely. This could mean using the
|
||||
/// Runtime from a single thread, using a mutex, doing all work on a
|
||||
|
@ -118,7 +118,14 @@ class JSI_EXPORT HostObject {
|
|||
/// argument. Destructors (all but ~Scope), operators, or other methods
|
||||
/// which do not take Runtime& as an argument are safe to call from any
|
||||
/// thread, but it is still forbidden to make write operations on a single
|
||||
/// instance of any class from more than one thread.
|
||||
/// instance of any class from more than one thread. In addition, to
|
||||
/// make shutdown safe, destruction of objects associated with the Runtime
|
||||
/// must be destroyed before the Runtime is destroyed, or from the
|
||||
/// destructor of a managed HostObject or HostFunction. Informally, this
|
||||
/// means that the main source of unsafe behavior is to hold a jsi object
|
||||
/// in a non-Runtime-managed object, and not clean it up before the Runtime
|
||||
/// is shut down. If your lifecycle is such that avoiding this is hard,
|
||||
/// you will probably need to do use your own locks.
|
||||
class Runtime {
|
||||
public:
|
||||
virtual ~Runtime();
|
||||
|
|
Загрузка…
Ссылка в новой задаче