Reviewed by Darin.
Disable JSLock for per-thread contexts.
No change on SunSpider.
* kjs/JSGlobalData.h:
* kjs/JSGlobalData.cpp:
(KJS::JSGlobalData::JSGlobalData):
(KJS::JSGlobalData::sharedInstance):
Added isSharedInstance as a better way to tell whether the instance is shared (legacy).
* kjs/JSLock.cpp:
(KJS::createJSLockCount):
(KJS::JSLock::lockCount):
(KJS::setLockCount):
(KJS::JSLock::JSLock):
(KJS::JSLock::lock):
(KJS::JSLock::unlock):
(KJS::JSLock::currentThreadIsHoldingLock):
(KJS::JSLock::DropAllLocks::DropAllLocks):
(KJS::JSLock::DropAllLocks::~DropAllLocks):
* kjs/JSLock.h:
(KJS::JSLock::JSLock):
(KJS::JSLock::~JSLock):
Made JSLock and JSLock::DropAllLocks constructors take a parameter to decide whether to
actually lock a mutex, or only to increment recursion count. We cannot turn it into no-op
if we want to keep existing assertions working.
Made recursion count per-thread, now that locks may not lock.
* API/JSBase.cpp:
(JSEvaluateScript): Take JSLock after casting JSContextRef to ExecState* (which doesn't need
locking in any case), so that a decision whether to actually lock can be made.
(JSCheckScriptSyntax): Ditto.
(JSGarbageCollect): Only lock while collecting the shared heap, not the per-thread one.
* API/JSObjectRef.cpp:
(JSClassCreate): Don't lock, as there is no reason to.
(JSClassRetain): Ditto.
(JSClassRelease): Ditto.
(JSPropertyNameArrayRetain): Ditto.
(JSPropertyNameArrayRelease): Only lock while deleting the array, as that may touch
identifier table.
(JSPropertyNameAccumulatorAddName): Adding a string also involves an identifier table
lookup, and possibly modification.
* API/JSStringRef.cpp:
(JSStringCreateWithCharacters):
(JSStringCreateWithUTF8CString):
(JSStringRetain):
(JSStringRelease):
(JSStringGetUTF8CString):
(JSStringIsEqual):
* API/JSStringRefCF.cpp:
(JSStringCreateWithCFString):
JSStringRef operations other than releasing do not need locking.
* VM/Machine.cpp: Don't include unused JSLock.h.
* kjs/CollectorHeapIntrospector.cpp: (KJS::CollectorHeapIntrospector::statistics):
Don't take the lock for real, as heap introspection pauses the process anyway. It seems that
the existing code could cause deadlocks.
* kjs/Shell.cpp:
(functionGC):
(main):
(jscmain):
The test tool uses a per-thread context, so no real locking is required.
* kjs/collector.h:
(KJS::Heap::setGCProtectNeedsLocking): Optionally protect m_protectedValues access with a
per-heap mutex. This is only needed for WebCore Database code, which violates the "no data
migration between threads" by using ProtectedPtr on a background thread.
(KJS::Heap::isShared): Keep a shared flag here, as well.
* kjs/protect.h:
(KJS::::ProtectedPtr):
(KJS::::~ProtectedPtr):
(KJS::::operator):
(KJS::operator==):
(KJS::operator!=):
ProtectedPtr is ony used from WebCore, so it doesn't need to take JSLock. An assertion in
Heap::protect/unprotect guards agains possible future unlocked uses of ProtectedPtr in JSC.
* kjs/collector.cpp:
(KJS::Heap::Heap): Initialize m_isShared.
(KJS::Heap::~Heap): No need to lock for real during destruction, but must keep assertions
in sweep() working.
(KJS::destroyRegisteredThread): Registered thread list is only accessed for shared heap,
so locking is always needed here.
(KJS::Heap::registerThread): Ditto.
(KJS::Heap::markStackObjectsConservatively): Use m_isShared instead of comparing to a shared
instance for a small speedup.
(KJS::Heap::setGCProtectNeedsLocking): Create m_protectedValuesMutex. There is currently no
way to undo this - and ideally, Database code will be fixed to lo longer require this quirk.
(KJS::Heap::protect): Take m_protectedValuesMutex (if it exists) while accessing
m_protectedValues.
(KJS::Heap::unprotect): Ditto.
(KJS::Heap::markProtectedObjects): Ditto.
(KJS::Heap::protectedGlobalObjectCount): Ditto.
(KJS::Heap::protectedObjectCount): Ditto.
(KJS::Heap::protectedObjectTypeCounts): Ditto.
* kjs/ustring.cpp:
* kjs/ustring.h:
Don't include JSLock.h, which is no longer used here. As a result, an explicit include had
to be added to many files in JavaScriptGlue, WebCore and WebKit.
* kjs/JSGlobalObject.cpp:
(KJS::JSGlobalObject::init):
* API/JSCallbackConstructor.cpp:
(KJS::constructJSCallback):
* API/JSCallbackFunction.cpp:
(KJS::JSCallbackFunction::call):
* API/JSCallbackObjectFunctions.h:
(KJS::::init):
(KJS::::getOwnPropertySlot):
(KJS::::put):
(KJS::::deleteProperty):
(KJS::::construct):
(KJS::::hasInstance):
(KJS::::call):
(KJS::::getPropertyNames):
(KJS::::toNumber):
(KJS::::toString):
(KJS::::staticValueGetter):
(KJS::::callbackGetter):
* API/JSContextRef.cpp:
(JSGlobalContextCreate):
(JSGlobalContextRetain):
(JSGlobalContextRelease):
* API/JSValueRef.cpp:
(JSValueIsEqual):
(JSValueIsStrictEqual):
(JSValueIsInstanceOfConstructor):
(JSValueMakeNumber):
(JSValueMakeString):
(JSValueToNumber):
(JSValueToStringCopy):
(JSValueToObject):
(JSValueProtect):
(JSValueUnprotect):
* JavaScriptCore.exp:
* kjs/PropertyNameArray.h:
(KJS::PropertyNameArray::globalData):
* kjs/interpreter.cpp:
(KJS::Interpreter::checkSyntax):
(KJS::Interpreter::evaluate):
Pass a parameter to JSLock/JSLock::DropAllLocks to decide whether the lock needs to be taken.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@34947 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/JavaScriptCore/kjs/collector.cpp b/JavaScriptCore/kjs/collector.cpp
index e3846c9..65b6bf5 100644
--- a/JavaScriptCore/kjs/collector.cpp
+++ b/JavaScriptCore/kjs/collector.cpp
@@ -23,6 +23,7 @@
#include "ExecState.h"
#include "JSGlobalObject.h"
+#include "JSLock.h"
#include "JSString.h"
#include "JSValue.h"
#include "list.h"
@@ -90,8 +91,9 @@
static void freeHeap(CollectorHeap*);
-Heap::Heap()
+Heap::Heap(bool isShared)
: m_markListSet(0)
+ , m_isShared(isShared)
{
memset(&primaryHeap, 0, sizeof(CollectorHeap));
memset(&numberHeap, 0, sizeof(CollectorHeap));
@@ -99,7 +101,7 @@
Heap::~Heap()
{
- JSLock lock;
+ JSLock lock(false);
delete m_markListSet;
sweep<PrimaryHeap>();
@@ -438,7 +440,9 @@
// Can't use JSLock convenience object here because we don't want to re-register
// an exiting thread.
- JSLock::lock();
+ // JSLock is only used for code simplicity here, as we only need to protect registeredThreads
+ // list manipulation, not JS data structures.
+ JSLock::lock(true);
if (registeredThreads == thread) {
registeredThreads = registeredThreads->next;
@@ -455,7 +459,7 @@
ASSERT(t); // If t is NULL, we never found ourselves in the list.
}
- JSLock::unlock();
+ JSLock::unlock(true);
delete thread;
}
@@ -467,6 +471,7 @@
void Heap::registerThread()
{
+ // Since registerThread() is only called when using a shared heap, these locks will be real.
ASSERT(JSLock::lockCount() > 0);
ASSERT(JSLock::currentThreadIsHoldingLock());
@@ -728,7 +733,7 @@
#if USE(MULTIPLE_THREADS)
- if (this == JSGlobalData::sharedInstance().heap) {
+ if (m_isShared) {
#ifndef NDEBUG
// Forbid malloc during the mark phase. Marking a thread suspends it, so
@@ -736,6 +741,8 @@
// suspended while holding the malloc lock.
fastMallocForbid();
#endif
+ // It is safe to access the registeredThreads list, because we earlier asserted that locks are being held,
+ // and since this is a shared heap, they are real locks.
for (Thread* thread = registeredThreads; thread != NULL; thread = thread->next) {
if (!pthread_equal(thread->posixThread, pthread_self()))
markOtherThreadConservatively(thread);
@@ -747,28 +754,48 @@
#endif
}
+void Heap::setGCProtectNeedsLocking()
+{
+ // Most clients do not need to call this, with the notable exception of WebCore.
+ // Clients that use shared heap have JSLock protection, while others are not supposed
+ // to migrate JS objects between threads. WebCore violates this contract in Database code,
+ // which calls gcUnprotect from a secondary thread.
+ if (!m_protectedValuesMutex)
+ m_protectedValuesMutex.set(new Mutex);
+}
+
void Heap::protect(JSValue* k)
{
ASSERT(k);
- ASSERT(JSLock::lockCount() > 0);
- ASSERT(JSLock::currentThreadIsHoldingLock());
+ ASSERT(JSLock::currentThreadIsHoldingLock() || !m_isShared);
if (JSImmediate::isImmediate(k))
return;
- protectedValues.add(k->asCell());
+ if (m_protectedValuesMutex)
+ m_protectedValuesMutex->lock();
+
+ m_protectedValues.add(k->asCell());
+
+ if (m_protectedValuesMutex)
+ m_protectedValuesMutex->unlock();
}
void Heap::unprotect(JSValue* k)
{
ASSERT(k);
- ASSERT(JSLock::lockCount() > 0);
- ASSERT(JSLock::currentThreadIsHoldingLock());
+ ASSERT(JSLock::currentThreadIsHoldingLock() || !m_isShared);
if (JSImmediate::isImmediate(k))
return;
- protectedValues.remove(k->asCell());
+ if (m_protectedValuesMutex)
+ m_protectedValuesMutex->lock();
+
+ m_protectedValues.remove(k->asCell());
+
+ if (m_protectedValuesMutex)
+ m_protectedValuesMutex->unlock();
}
Heap* Heap::heap(const JSValue* v)
@@ -780,12 +807,18 @@
void Heap::markProtectedObjects()
{
- ProtectCountSet::iterator end = protectedValues.end();
- for (ProtectCountSet::iterator it = protectedValues.begin(); it != end; ++it) {
+ if (m_protectedValuesMutex)
+ m_protectedValuesMutex->lock();
+
+ ProtectCountSet::iterator end = m_protectedValues.end();
+ for (ProtectCountSet::iterator it = m_protectedValues.begin(); it != end; ++it) {
JSCell* val = it->first;
if (!val->marked())
val->mark();
}
+
+ if (m_protectedValuesMutex)
+ m_protectedValuesMutex->unlock();
}
template <Heap::HeapType heapType> size_t Heap::sweep()
@@ -940,21 +973,36 @@
size_t Heap::protectedGlobalObjectCount()
{
+ if (m_protectedValuesMutex)
+ m_protectedValuesMutex->lock();
+
size_t count = 0;
if (JSGlobalObject* head = JSGlobalData::threadInstance().head) {
JSGlobalObject* o = head;
do {
- if (protectedValues.contains(o))
+ if (m_protectedValues.contains(o))
++count;
o = o->next();
} while (o != head);
}
+
+ if (m_protectedValuesMutex)
+ m_protectedValuesMutex->unlock();
+
return count;
}
size_t Heap::protectedObjectCount()
{
- return protectedValues.size();
+ if (m_protectedValuesMutex)
+ m_protectedValuesMutex->lock();
+
+ size_t result = m_protectedValues.size();
+
+ if (m_protectedValuesMutex)
+ m_protectedValuesMutex->unlock();
+
+ return result;
}
static const char* typeName(JSCell* val)
@@ -994,10 +1042,16 @@
{
HashCountedSet<const char*>* counts = new HashCountedSet<const char*>;
- ProtectCountSet::iterator end = protectedValues.end();
- for (ProtectCountSet::iterator it = protectedValues.begin(); it != end; ++it)
+ if (m_protectedValuesMutex)
+ m_protectedValuesMutex->lock();
+
+ ProtectCountSet::iterator end = m_protectedValues.end();
+ for (ProtectCountSet::iterator it = m_protectedValues.begin(); it != end; ++it)
counts->add(typeName(it->first));
+ if (m_protectedValuesMutex)
+ m_protectedValuesMutex->unlock();
+
return counts;
}
