Unreviewed, rolling out r250583.
authortsavell@apple.com <tsavell@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Oct 2019 15:35:03 +0000 (15:35 +0000)
committertsavell@apple.com <tsavell@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 4 Oct 2019 15:35:03 +0000 (15:35 +0000)
Broke multiple internal API tests

Reverted changeset:

"[JSC] Place VM* in TLS"
https://bugs.webkit.org/show_bug.cgi?id=202391
https://trac.webkit.org/changeset/250583

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@250725 268f45cc-cd09-0410-ab3c-d52691b4dbfc

13 files changed:
Source/JavaScriptCore/API/JSManagedValue.mm
Source/JavaScriptCore/API/glib/JSCWeakValue.cpp
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/runtime/InitializeThreading.cpp
Source/JavaScriptCore/runtime/JSLock.cpp
Source/JavaScriptCore/runtime/JSLock.h
Source/JavaScriptCore/runtime/JSRunLoopTimer.cpp
Source/JavaScriptCore/runtime/VM.cpp
Source/JavaScriptCore/runtime/VM.h
Source/WTF/ChangeLog
Source/WTF/wtf/FastTLS.h
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/IDBBindingUtilities.cpp

index deea6a5..9f5dd88 100644 (file)
@@ -147,9 +147,12 @@ static JSManagedValueHandleOwner& managedValueHandleOwner()
 
 - (JSValue *)value
 {
-    JSC::JSLockHolder locker(JSC::JSLockHolder::LockIfVMIsLive, *m_lock.get());
-    if (!locker.vm())
+    WTF::Locker<JSC::JSLock> locker(m_lock.get());
+    JSC::VM* vm = m_lock->vm();
+    if (!vm)
         return nil;
+
+    JSC::JSLockHolder apiLocker(vm);
     if (!m_globalObject)
         return nil;
     if (m_weakValue.isClear())
index e147608..9897ee3 100644 (file)
@@ -189,10 +189,12 @@ JSCValue* jsc_weak_value_get_value(JSCWeakValue* weakValue)
     g_return_val_if_fail(JSC_IS_WEAK_VALUE(weakValue), nullptr);
 
     JSCWeakValuePrivate* priv = weakValue->priv;
-    JSC::JSLockHolder locker(JSC::JSLockHolder::LockIfVMIsLive, *priv->lock.get());
-    if (!locker.vm())
+    WTF::Locker<JSC::JSLock> locker(priv->lock.get());
+    JSC::VM* vm = priv->lock->vm();
+    if (!vm)
         return nullptr;
 
+    JSC::JSLockHolder apiLocker(vm);
     if (!priv->globalObject || priv->weakValueRef.isClear())
         return nullptr;
 
index 477bef1..45133b9 100644 (file)
@@ -1,5 +1,17 @@
 2019-10-04  Truitt Savell  <tsavell@apple.com>
 
+        Unreviewed, rolling out r250583.
+
+        Broke multiple internal API tests
+
+        Reverted changeset:
+
+        "[JSC] Place VM* in TLS"
+        https://bugs.webkit.org/show_bug.cgi?id=202391
+        https://trac.webkit.org/changeset/250583
+
+2019-10-04  Truitt Savell  <tsavell@apple.com>
+
         Unreviewed, rolling out r250594.
 
         Broke multiple internal API tests
index 4abea2a..1fd59f4 100644 (file)
@@ -71,8 +71,6 @@ void initializeThreading()
 
         initializePtrTagLookup();
 
-        VM::initializeTLS();
-
 #if ENABLE(WRITE_BARRIER_PROFILING)
         WriteBarrierCounters::initialize();
 #endif
index ad4d0d1..daf0f77 100644 (file)
@@ -28,7 +28,6 @@
 #include "JSCInlines.h"
 #include "MachineStackMarker.h"
 #include "SamplingProfiler.h"
-#include "VM.h"
 #include "WasmCapabilities.h"
 #include "WasmMachineThreads.h"
 #include <thread>
@@ -68,27 +67,13 @@ JSLockHolder::JSLockHolder(VM& vm)
     : m_vm(&vm)
 {
     m_vm->apiLock().lock();
-    m_previousVMInTLS = VM::exchange(m_vm.get());
-}
-
-JSLockHolder::JSLockHolder(LockIfVMIsLiveTag, JSLock& lock)
-{
-    lock.lock();
-    if (lock.m_vm) {
-        m_vm = lock.m_vm;
-        m_previousVMInTLS = VM::exchange(m_vm.get());
-    } else
-        lock.unlock();
 }
 
 JSLockHolder::~JSLockHolder()
 {
-    if (m_vm) {
-        RefPtr<JSLock> apiLock(&m_vm->apiLock());
-        m_vm = nullptr;
-        apiLock->unlock();
-        VM::exchange(m_previousVMInTLS); // Change VM only after unlocking. Otherwise, willReleaseLock will see previous VM.
-    }
+    RefPtr<JSLock> apiLock(&m_vm->apiLock());
+    m_vm = nullptr;
+    apiLock->unlock();
 }
 
 JSLock::JSLock(VM* vm)
@@ -237,6 +222,16 @@ void JSLock::willReleaseLock()
     }
 }
 
+void JSLock::lock(ExecState* exec)
+{
+    exec->vm().apiLock().lock();
+}
+
+void JSLock::unlock(ExecState* exec)
+{
+    exec->vm().apiLock().unlock();
+}
+
 // This function returns the number of locks that were dropped.
 unsigned JSLock::dropAllLocks(DropAllLocks* dropper)
 {
@@ -290,8 +285,6 @@ JSLock::DropAllLocks::DropAllLocks(VM* vm)
         return;
     RELEASE_ASSERT(!m_vm->apiLock().currentThreadIsHoldingLock() || !m_vm->isCollectorBusyOnCurrentThread());
     m_droppedLockCount = m_vm->apiLock().dropAllLocks(this);
-    VM* previousVMInTLS = VM::exchange(nullptr);
-    ASSERT_UNUSED(previousVMInTLS, previousVMInTLS == m_vm.get());
 }
 
 JSLock::DropAllLocks::DropAllLocks(ExecState* exec)
@@ -309,8 +302,6 @@ JSLock::DropAllLocks::~DropAllLocks()
     if (!m_vm)
         return;
     m_vm->apiLock().grabAllLocks(this, m_droppedLockCount);
-    VM* previousVMInTLS = VM::exchange(m_vm.get());
-    ASSERT_UNUSED(previousVMInTLS, previousVMInTLS);
 }
 
 } // namespace JSC
index 39934a6..0b27ff6 100644 (file)
@@ -51,7 +51,6 @@ namespace JSC {
 
 class ExecState;
 class VM;
-class JSLock;
 
 // This class is used to protect the initialization of the legacy single 
 // shared VM.
@@ -64,34 +63,32 @@ private:
     static Lock s_sharedInstanceMutex;
 };
 
-// Only JSLockHolder can take a JSLock.
 class JSLockHolder {
-    WTF_MAKE_NONCOPYABLE(JSLockHolder);
-    WTF_FORBID_HEAP_ALLOCATION(JSLockHolder);
 public:
     JS_EXPORT_PRIVATE JSLockHolder(VM*);
     JS_EXPORT_PRIVATE JSLockHolder(VM&);
     JS_EXPORT_PRIVATE JSLockHolder(ExecState*);
 
-    enum LockIfVMIsLiveTag { LockIfVMIsLive };
-    JS_EXPORT_PRIVATE JSLockHolder(LockIfVMIsLiveTag, JSLock&);
-
     JS_EXPORT_PRIVATE ~JSLockHolder();
 
-    VM* vm() { return m_vm.get(); }
-
 private:
     RefPtr<VM> m_vm;
-    VM* m_previousVMInTLS { nullptr };
 };
 
 class JSLock : public ThreadSafeRefCounted<JSLock> {
     WTF_MAKE_NONCOPYABLE(JSLock);
-    friend class JSLockHolder;
 public:
     JSLock(VM*);
     JS_EXPORT_PRIVATE ~JSLock();
 
+    JS_EXPORT_PRIVATE void lock();
+    JS_EXPORT_PRIVATE void unlock();
+
+    static void lock(ExecState*);
+    static void unlock(ExecState*);
+    static void lock(VM&);
+    static void unlock(VM&);
+
     VM* vm() { return m_vm; }
 
     Optional<RefPtr<Thread>> ownerThread() const
@@ -129,10 +126,6 @@ public:
     bool isWebThreadAware() const { return m_isWebThreadAware; }
 
 private:
-    // This is called only from JSLockHolder or DropAllLocks.
-    JS_EXPORT_PRIVATE void lock();
-    JS_EXPORT_PRIVATE void unlock();
-
     void lock(intptr_t lockCount);
     void unlock(intptr_t unlockCount);
 
index e60c349..2a0c06c 100644 (file)
@@ -297,12 +297,14 @@ void JSRunLoopTimer::timerDidFire()
         }
     }
 
-    JSLockHolder locker(JSLockHolder::LockIfVMIsLive, m_apiLock.get());
-    if (!locker.vm()) {
+    std::lock_guard<JSLock> lock(m_apiLock.get());
+    RefPtr<VM> vm = m_apiLock->vm();
+    if (!vm) {
         // The VM has been destroyed, so we should just give up.
         return;
     }
-    doWork(*locker.vm());
+
+    doWork(*vm);
 }
 
 JSRunLoopTimer::JSRunLoopTimer(VM& vm)
index 091cfa9..adb5a91 100644 (file)
@@ -1379,16 +1379,4 @@ void VM::setCrashOnVMCreation(bool shouldCrash)
     vmCreationShouldCrash = shouldCrash;
 }
 
-
-#if !HAVE(FAST_TLS)
-WTF::ThreadSpecificKey VM::tlsKey { WTF::InvalidThreadSpecificKey };
-#endif
-
-void VM::initializeTLS()
-{
-#if !HAVE(FAST_TLS)
-    WTF::threadSpecificKeyCreate(&tlsKey, nullptr);
-#endif
-}
-
 } // namespace JSC
index 676d9c2..39bea5d 100644 (file)
@@ -314,39 +314,6 @@ public:
     WeakRandom& random() { return m_random; }
     Integrity::Random& integrityRandom() { return m_integrityRandom; }
 
-#if HAVE(FAST_TLS)
-    static constexpr pthread_key_t tlsKey = WTF_VM_KEY;
-
-    static VM* exchange(VM* vm)
-    {
-        VM* previous = current();
-        _pthread_setspecific_direct(tlsKey, bitwise_cast<void*>(vm));
-        return previous;
-    }
-
-    static VM* current()
-    {
-        return bitwise_cast<VM*>(_pthread_getspecific_direct(tlsKey));
-    }
-#else
-    static WTF::ThreadSpecificKey tlsKey;
-
-    static VM* exchange(VM* vm)
-    {
-        ASSERT(tlsKey != WTF::InvalidThreadSpecificKey);
-        VM* previous = current();
-        WTF::threadSpecificSet(tlsKey, vm);
-        return previous;
-    }
-
-    static VM* current()
-    {
-        ASSERT(tlsKey != WTF::InvalidThreadSpecificKey);
-        return bitwise_cast<VM*>(WTF::threadSpecificGet(tlsKey));
-    }
-#endif
-    static void initializeTLS();
-
 private:
     unsigned nextID();
 
index 2397155..6f18fde 100644 (file)
@@ -1,3 +1,15 @@
+2019-10-04  Truitt Savell  <tsavell@apple.com>
+
+        Unreviewed, rolling out r250583.
+
+        Broke multiple internal API tests
+
+        Reverted changeset:
+
+        "[JSC] Place VM* in TLS"
+        https://bugs.webkit.org/show_bug.cgi?id=202391
+        https://trac.webkit.org/changeset/250583
+
 2019-10-03  Keith Rollin  <krollin@apple.com>
 
         Remove some support for < iOS 13
index 7ac0a80..fe1cdd9 100644 (file)
@@ -46,7 +46,7 @@ namespace WTF {
 #define WTF_THREAD_DATA_KEY WTF_FAST_TLS_KEY0
 #define WTF_WASM_CONTEXT_KEY WTF_FAST_TLS_KEY1
 #define WTF_TESTING_KEY WTF_WASM_CONTEXT_KEY // So far, this key is only used in places that don't do WebAssembly, so it's OK that they share the same key.
-#define WTF_VM_KEY WTF_FAST_TLS_KEY2
+#define WTF_GC_TLC_KEY WTF_FAST_TLS_KEY2
 
 #if ENABLE(FAST_TLS_JIT)
 inline unsigned fastTLSOffsetForKey(unsigned long slot)
index ad7b3f8..4d864dc 100644 (file)
@@ -1,3 +1,15 @@
+2019-10-04  Truitt Savell  <tsavell@apple.com>
+
+        Unreviewed, rolling out r250583.
+
+        Broke multiple internal API tests
+
+        Reverted changeset:
+
+        "[JSC] Place VM* in TLS"
+        https://bugs.webkit.org/show_bug.cgi?id=202391
+        https://trac.webkit.org/changeset/250583
+
 2019-10-04  Alex Christensen  <achristensen@webkit.org>
 
         Simplify sandbox enabling macros
index 5a9ccf4..85e5e9f 100644 (file)
@@ -133,7 +133,7 @@ JSValue toJS(ExecState& state, JSGlobalObject& globalObject, IDBKey* key)
     }
 
     VM& vm = state.vm();
-    JSLockHolder locker(vm);
+    Locker<JSLock> locker(vm.apiLock());
     auto scope = DECLARE_THROW_SCOPE(vm);
 
     switch (key->type()) {
@@ -386,9 +386,10 @@ static JSValue deserializeIDBValueToJSValue(ExecState& state, JSC::JSGlobalObjec
 
     auto serializedValue = SerializedScriptValue::createFromWireBytes(Vector<uint8_t>(data));
 
-    JSLockHolder locker(globalObject.vm());
+    state.vm().apiLock().lock();
     Vector<RefPtr<MessagePort>> messagePorts;
     JSValue result = serializedValue->deserialize(state, &globalObject, messagePorts, value.blobURLs(), value.blobFilePaths(), SerializationErrorMode::NonThrowing);
+    state.vm().apiLock().unlock();
 
     return result;
 }