Reviewed by Maciej Stachowiak.
Fixed all known crashers exposed by run-webkit-tests --threaded. This covers:
<rdar://problem/
4565394> | http://bugs.webkit.org/show_bug.cgi?id=12585
PAC file: after closing a window that contains macworld.com, new window
crashes (KJS::PropertyMap::mark()) (12585)
<rdar://problem/
4571215> | http://bugs.webkit.org/show_bug.cgi?id=9211
PAC file: Crash occurs when clicking on the navigation tabs at http://www.businessweek.com/ (9211)
<rdar://problem/
4557926>
PAC file: Crash occurs when attempting to view image in slideshow mode
at http://d.smugmug.com/gallery/581716 ( KJS::IfNode::execute (KJS::
ExecState*) + 312) if you use a PAC file
(1) Added some missing JSLocks, along with related ASSERTs.
(2) Fully implemented support for objects that can only be garbage collected
on the main thread. So far, only WebCore uses this. We can add it to API
later if we learn that it's needed.
The implementation uses a "main thread only" flag inside each object. When
collecting on a secondary thread, the Collector does an extra pass through
the heap to mark all flagged objects before sweeping. This solution makes
the common case -- flag lots of objects, but never collect on a secondary
thread -- very fast, even though the uncommon case of garbage collecting
on a secondary thread isn't as fast as it could be. I left some notes
about how to speed it up, if we ever care.
For posterity, here are some things I learned about GC while investigating:
* Each collect must either mark or delete every heap object. "Zombie"
objects, which are neither marked nor deleted, raise these issues:
* On the next pass, the conservative marking algorithm might mark a
zombie, causing it to mark freed objects.
* The client might try to use a zombie, which would seem live because
its finalizer had not yet run.
* A collect on the main thread is free to delete any object. Presumably,
objects allocated on secondary threads have thread-safe finalizers.
* A collect on a secondary thread must not delete thread-unsafe objects.
* The mark function must be thread-safe.
Line by line comments:
* API/JSObjectRef.h: Added comment specifying that the finalize callback
may run on any thread.
* JavaScriptCore.exp: Nothing to see here.
* bindings/npruntime.cpp:
(_NPN_GetStringIdentifier): Added JSLock.
* bindings/objc/objc_instance.h:
* bindings/objc/objc_instance.mm:
(ObjcInstance::~ObjcInstance): Use an autorelease pool. The other callers
to CFRelease needed one, too, but they were dead code, so I removed them
instead. (This fixes a leak seen while running run-webkit-tests --threaded,
although I don't think it's specifically a threading issue.)
* kjs/collector.cpp:
(KJS::Collector::collectOnMainThreadOnly): New function. Tells the collector
to collect a value only if it's collecting on the main thread.
(KJS::Collector::markMainThreadOnlyObjects): New function. Scans the heap
for "main thread only" objects and marks them.
* kjs/date_object.cpp:
(KJS::DateObjectImp::DateObjectImp): To make the new ASSERTs happy, allocate
our globals on the heap, avoiding a seemingly unsafe destructor call at
program exit time.
* kjs/function_object.cpp:
(FunctionPrototype::FunctionPrototype): ditto
* kjs/interpreter.cpp:
(KJS::Interpreter::mark): Removed boolean parameter, which was an incomplete
and arguably hackish way to implement markMainThreadOnlyObjects() inside WebCore.
* kjs/interpreter.h:
* kjs/identifier.cpp:
(KJS::identifierTable): Added some ASSERTs to check for thread safety
problems.
* kjs/list.cpp: Added some ASSERTs to check for thread safety problems.
(KJS::allocateListImp):
(KJS::List::release):
(KJS::List::append):
(KJS::List::empty): Make the new ASSERTs happy.
* kjs/object.h:
(KJS::JSObject::JSObject): "m_destructorIsThreadSafe" => "m_collectOnMainThreadOnly".
I removed the constructor parameter because m_collectOnMainThreadOnly,
like m_marked, is a Collector bit, so only the Collector should set or get it.
* kjs/object_object.cpp:
(ObjectPrototype::ObjectPrototype): Make the ASSERTs happy.
* kjs/regexp_object.cpp:
(RegExpPrototype::RegExpPrototype): ditto
* kjs/ustring.cpp: Added some ASSERTs to check for thread safety problems.
(KJS::UCharReference::ref):
(KJS::UString::Rep::createCopying):
(KJS::UString::Rep::create):
(KJS::UString::Rep::destroy):
(KJS::UString::null): Make the new ASSERTs happy.
* kjs/ustring.h:
(KJS::UString::Rep::ref): Added some ASSERTs to check for thread safety problems.
(KJS::UString::Rep::deref):
* kjs/value.h:
(KJS::JSCell::JSCell):
JavaScriptGlue:
Reviewed by Maciej Stachowiak.
Fixed all known crashers exposed by run-webkit-tests --threaded while using
a PAC file (for maximum carnage). See JavaScriptCore ChangeLog for
more details.
* JSBase.cpp:
(JSBase::Release): Lock when deleting, because we may be deleting an object
(like a JSRun) that holds thread-unsafe data.
* JSUtils.cpp:
(CFStringToUString): Don't lock, because our caller locks. Also, locking
inside a function that returns thread-unsafe data by copy will only mask
threading problems.
* JavaScriptGlue.cpp:
(JSRunEvaluate): Added missing JSLock.
(JSRunCheckSyntax): Converted to JSLock.
* JavaScriptGlue.xcodeproj/project.pbxproj:
WebCore:
Reviewed by Maciej Stachowiak.
Fixed all known crashers exposed by run-webkit-tests --threaded [*]. See
JavaScriptCore ChangeLog for more details.
* bindings/js/kjs_binding.cpp:
(KJS::domNodesPerDocument): Added thread safety ASSERT.
(KJS::ScriptInterpreter::mark): Removed obsolete logic for marking unsafe
objects when collecting on a secondary thread. The Collector takes care
of this now.
* bindings/js/kjs_binding.h:
(KJS::DOMObject::DOMObject): Used new API for specifying that WebCore
objects should be garbage collected on the main thread only.
* bindings/js/kjs_window.cpp:
(KJS::ScheduledAction::execute): Moved JSLock to cover implementedsCall() call,
which, for some subclasses, ends up allocating garbage collected objects.
(This fix was speculative. I didn't actually see a crash from this.)
(KJS::Window::timerFired): Added JSLock around ScheduleAction destruction,
since it destroys a KJS::List.
* bindings/objc/WebScriptObject.mm:
(-[WebScriptObject setException:]): Added JSLock. (This fix was speculative.
I didn't actually see a crash from this.)
* bridge/mac/WebCoreScriptDebugger.mm:
(-[WebCoreScriptCallFrame evaluateWebScript:]): Added JSLock. (This fix
was speculative. I didn't actually see a crash from this.)
* dom/Document.cpp:
(WebCore::Document::~Document): Added JSLock around modification to
domNodesPerDocument(), which can be accessed concurrently during garbage
collection.
* dom/Node.cpp:
(WebCore::Node::setDocument): ditto.
[*] fast/js/toString-stack-overflow.html is an exception. --threaded mode
crashes this test because it causes the garbage collector to run frequently,
and this test crashes if you happen to garbage collect while it's running.
This is a known issue with stack overflow during the mark phase. It's
not related to threading.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@20004
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
/*!
@typedef JSObjectFinalizeCallback
-@abstract The callback invoked when an object is finalized (prepared for garbage collection).
+@abstract The callback invoked when an object is finalized (prepared for garbage collection). An object may be finalized on any thread.
@param object The JSObject being finalized.
@discussion If you named your function Finalize, you would declare it like this:
+2007-03-06 Geoffrey Garen <ggaren@apple.com>
+
+ Reviewed by Maciej Stachowiak.
+
+ Fixed all known crashers exposed by run-webkit-tests --threaded. This covers:
+
+ <rdar://problem/4565394> | http://bugs.webkit.org/show_bug.cgi?id=12585
+ PAC file: after closing a window that contains macworld.com, new window
+ crashes (KJS::PropertyMap::mark()) (12585)
+ <rdar://problem/4571215> | http://bugs.webkit.org/show_bug.cgi?id=9211
+ PAC file: Crash occurs when clicking on the navigation tabs at http://www.businessweek.com/ (9211)
+ <rdar://problem/4557926>
+ PAC file: Crash occurs when attempting to view image in slideshow mode
+ at http://d.smugmug.com/gallery/581716 ( KJS::IfNode::execute (KJS::
+ ExecState*) + 312) if you use a PAC file
+
+ (1) Added some missing JSLocks, along with related ASSERTs.
+
+ (2) Fully implemented support for objects that can only be garbage collected
+ on the main thread. So far, only WebCore uses this. We can add it to API
+ later if we learn that it's needed.
+
+ The implementation uses a "main thread only" flag inside each object. When
+ collecting on a secondary thread, the Collector does an extra pass through
+ the heap to mark all flagged objects before sweeping. This solution makes
+ the common case -- flag lots of objects, but never collect on a secondary
+ thread -- very fast, even though the uncommon case of garbage collecting
+ on a secondary thread isn't as fast as it could be. I left some notes
+ about how to speed it up, if we ever care.
+
+ For posterity, here are some things I learned about GC while investigating:
+
+ * Each collect must either mark or delete every heap object. "Zombie"
+ objects, which are neither marked nor deleted, raise these issues:
+
+ * On the next pass, the conservative marking algorithm might mark a
+ zombie, causing it to mark freed objects.
+
+ * The client might try to use a zombie, which would seem live because
+ its finalizer had not yet run.
+
+ * A collect on the main thread is free to delete any object. Presumably,
+ objects allocated on secondary threads have thread-safe finalizers.
+
+ * A collect on a secondary thread must not delete thread-unsafe objects.
+
+ * The mark function must be thread-safe.
+
+ Line by line comments:
+
+ * API/JSObjectRef.h: Added comment specifying that the finalize callback
+ may run on any thread.
+
+ * JavaScriptCore.exp: Nothing to see here.
+
+ * bindings/npruntime.cpp:
+ (_NPN_GetStringIdentifier): Added JSLock.
+
+ * bindings/objc/objc_instance.h:
+ * bindings/objc/objc_instance.mm:
+ (ObjcInstance::~ObjcInstance): Use an autorelease pool. The other callers
+ to CFRelease needed one, too, but they were dead code, so I removed them
+ instead. (This fixes a leak seen while running run-webkit-tests --threaded,
+ although I don't think it's specifically a threading issue.)
+
+ * kjs/collector.cpp:
+ (KJS::Collector::collectOnMainThreadOnly): New function. Tells the collector
+ to collect a value only if it's collecting on the main thread.
+ (KJS::Collector::markMainThreadOnlyObjects): New function. Scans the heap
+ for "main thread only" objects and marks them.
+
+ * kjs/date_object.cpp:
+ (KJS::DateObjectImp::DateObjectImp): To make the new ASSERTs happy, allocate
+ our globals on the heap, avoiding a seemingly unsafe destructor call at
+ program exit time.
+ * kjs/function_object.cpp:
+ (FunctionPrototype::FunctionPrototype): ditto
+
+ * kjs/interpreter.cpp:
+ (KJS::Interpreter::mark): Removed boolean parameter, which was an incomplete
+ and arguably hackish way to implement markMainThreadOnlyObjects() inside WebCore.
+ * kjs/interpreter.h:
+
+ * kjs/identifier.cpp:
+ (KJS::identifierTable): Added some ASSERTs to check for thread safety
+ problems.
+
+ * kjs/list.cpp: Added some ASSERTs to check for thread safety problems.
+ (KJS::allocateListImp):
+ (KJS::List::release):
+ (KJS::List::append):
+ (KJS::List::empty): Make the new ASSERTs happy.
+
+ * kjs/object.h:
+ (KJS::JSObject::JSObject): "m_destructorIsThreadSafe" => "m_collectOnMainThreadOnly".
+ I removed the constructor parameter because m_collectOnMainThreadOnly,
+ like m_marked, is a Collector bit, so only the Collector should set or get it.
+
+ * kjs/object_object.cpp:
+ (ObjectPrototype::ObjectPrototype): Make the ASSERTs happy.
+ * kjs/regexp_object.cpp:
+ (RegExpPrototype::RegExpPrototype): ditto
+
+ * kjs/ustring.cpp: Added some ASSERTs to check for thread safety problems.
+ (KJS::UCharReference::ref):
+ (KJS::UString::Rep::createCopying):
+ (KJS::UString::Rep::create):
+ (KJS::UString::Rep::destroy):
+ (KJS::UString::null): Make the new ASSERTs happy.
+ * kjs/ustring.h:
+ (KJS::UString::Rep::ref): Added some ASSERTs to check for thread safety problems.
+ (KJS::UString::Rep::deref):
+
+ * kjs/value.h:
+ (KJS::JSCell::JSCell):
+
2007-03-06 Geoffrey Garen <ggaren@apple.com>
Reviewed by Maciej Stachowiak.
__ZN3KJS11Interpreter17startTimeoutCheckEv
__ZN3KJS11Interpreter21shouldPrintExceptionsEv
__ZN3KJS11Interpreter24setShouldPrintExceptionsEb
-__ZN3KJS11Interpreter4markEb
+__ZN3KJS11Interpreter4markEv
__ZN3KJS11Interpreter6s_hookE
__ZN3KJS11Interpreter8evaluateERKNS_7UStringEiPKNS_5UCharEiPNS_7JSValueE
__ZN3KJS11Interpreter8evaluateERKNS_7UStringEiS3_PNS_7JSValueE
__ZN3KJS6JSLock12DropAllLocksD1Ev
__ZN3KJS6JSLock4lockEv
__ZN3KJS6JSLock6unlockEv
+__ZN3KJS6JSLock9lockCountEv
__ZN3KJS6Lookup9findEntryEPKNS_9HashTableERKNS_10IdentifierE
__ZN3KJS6Parser11prettyPrintERKNS_7UStringEPiPS1_
__ZN3KJS7CStringD1Ev
__ZN3KJS9Collector15numInterpretersEv
__ZN3KJS9Collector19numProtectedObjectsEv
__ZN3KJS9Collector20rootObjectTypeCountsEv
+__ZN3KJS9Collector23collectOnMainThreadOnlyEPNS_7JSValueE
__ZN3KJS9Collector4sizeEv
__ZN3KJS9Collector7collectEv
__ZN3KJS9Collector7protectEPNS_7JSValueE
__ZTVN3KJS14StringInstanceE
__ZTVN3KJS15JSWrapperObjectE
__ZTVN3KJS19InternalFunctionImpE
+__ZTVN3KJS6JSCellE
__ZTVN3KJS8JSObjectE
_kJSClassDefinitionEmpty
_kjs_pcre_compile
#include "npruntime_impl.h"
#include "npruntime_priv.h"
+#include "JSLock.h"
#include "c_utility.h"
#include "identifier.h"
#include <wtf/Assertions.h>
if (name) {
PrivateIdentifier* identifier = 0;
+ KJS::JSLock lock;
+
identifier = getStringIdentifierMap()->get(identifierFromNPIdentifier(name).ustring().rep());
if (identifier == 0) {
identifier = (PrivateIdentifier*)malloc(sizeof(PrivateIdentifier));
virtual Class *getClass() const;
- ObjcInstance(const ObjcInstance &other);
-
- ObjcInstance &operator=(const ObjcInstance &other);
-
virtual void begin();
virtual void end();
JSValue *booleanValue() const;
private:
+ ObjcInstance(const ObjcInstance& other);
+ ObjcInstance& operator=(const ObjcInstance& other);
+
ObjectStructPtr _instance;
mutable ObjcClass *_class;
ObjectStructPtr _pool;
ObjcInstance::~ObjcInstance()
{
+ begin(); // -finalizeForWebScript and -dealloc/-finalize may require autorelease pools.
if ([_instance respondsToSelector:@selector(finalizeForWebScript)])
[_instance performSelector:@selector(finalizeForWebScript)];
if (_instance)
CFRelease(_instance);
-}
-
-ObjcInstance::ObjcInstance(const ObjcInstance &other) : Instance()
-{
- _instance = other._instance;
- if (_instance)
- CFRetain(_instance);
- _class = other._class;
- _pool = 0;
- _beginCount = 0;
-}
-
-ObjcInstance &ObjcInstance::operator=(const ObjcInstance &other)
-{
- ObjectStructPtr _oldInstance = _instance;
- _instance = other._instance;
- if (_instance)
- CFRetain(_instance);
- if (_oldInstance)
- CFRelease(_oldInstance);
- // Classes are kept around forever.
- _class = other._class;
- return* this;
+ end();
}
void ObjcInstance::begin()
static CollectorHeap heap = {NULL, 0, 0, 0, NULL, 0, 0, 0, 0};
+size_t Collector::mainThreadOnlyObjectCount = 0;
bool Collector::memoryFull = false;
#ifndef NDEBUG
protectedValues().remove(k->downcast());
}
+void Collector::collectOnMainThreadOnly(JSValue* value)
+{
+ ASSERT(value);
+ ASSERT(JSLock::lockCount() > 0);
+
+ if (JSImmediate::isImmediate(value))
+ return;
+
+ JSCell* cell = value->downcast();
+ cell->m_collectOnMainThreadOnly = true;
+ ++mainThreadOnlyObjectCount;
+}
+
void Collector::markProtectedObjects()
{
ProtectCountSet& protectedValues = KJS::protectedValues();
}
}
+void Collector::markMainThreadOnlyObjects()
+{
+ ASSERT(!pthread_main_np());
+
+ // Optimization for clients that never register "main thread only" objects.
+ if (!mainThreadOnlyObjectCount)
+ return;
+
+ // FIXME: We can optimize this marking algorithm by keeping an exact set of
+ // "main thread only" objects when the "main thread only" object count is
+ // small. We don't want to keep an exact set all the time, because WebCore
+ // tends to create lots of "main thread only" objects, and all that set
+ // thrashing can be expensive.
+
+ size_t count = 0;
+
+ for (size_t block = 0; block < heap.usedBlocks; block++) {
+ ASSERT(count < mainThreadOnlyObjectCount);
+
+ CollectorBlock* curBlock = heap.blocks[block];
+ size_t minimumCellsToProcess = curBlock->usedCells;
+ for (size_t i = 0; (i < minimumCellsToProcess) & (i < CELLS_PER_BLOCK); i++) {
+ CollectorCell* cell = curBlock->cells + i;
+ if (cell->u.freeCell.zeroIfFree == 0)
+ ++minimumCellsToProcess;
+ else {
+ JSCell* imp = reinterpret_cast<JSCell*>(cell);
+ if (imp->m_collectOnMainThreadOnly) {
+ if(!imp->marked())
+ imp->mark();
+ if (++count == mainThreadOnlyObjectCount)
+ return;
+ }
+ }
+ }
+ }
+
+ for (size_t cell = 0; cell < heap.usedOversizeCells; cell++) {
+ ASSERT(count < mainThreadOnlyObjectCount);
+
+ JSCell* imp = reinterpret_cast<JSCell*>(heap.oversizeCells[cell]);
+ if (imp->m_collectOnMainThreadOnly) {
+ if (!imp->marked())
+ imp->mark();
+ if (++count == mainThreadOnlyObjectCount)
+ return;
+ }
+ }
+}
+
bool Collector::collect()
{
assert(JSLock::lockCount() > 0);
if (Interpreter::s_hook) {
Interpreter* scr = Interpreter::s_hook;
do {
- scr->mark(currentThreadIsMainThread);
+ scr->mark();
scr = scr->next;
} while (scr != Interpreter::s_hook);
}
markStackObjectsConservatively();
markProtectedObjects();
List::markProtectedLists();
+ if (!currentThreadIsMainThread)
+ markMainThreadOnlyObjects();
// SWEEP: delete everything with a zero refcount (garbage) and unmark everything else
JSCell *imp = reinterpret_cast<JSCell *>(cell);
if (imp->m_marked) {
imp->m_marked = false;
- } else if (currentThreadIsMainThread || imp->m_destructorIsThreadSafe) {
+ } else {
// special case for allocated but uninitialized object
- // (We don't need this check earlier because nothing prior this point assumes the object has a valid vptr.)
+ // (We don't need this check earlier because nothing prior this point
+ // assumes the object has a valid vptr.)
if (cell->u.freeCell.zeroIfFree == 0)
continue;
+ ASSERT(currentThreadIsMainThread || !imp->m_collectOnMainThreadOnly);
+ if (imp->m_collectOnMainThreadOnly)
+ --mainThreadOnlyObjectCount;
imp->~JSCell();
--usedCells;
--numLiveObjects;
JSCell *imp = reinterpret_cast<JSCell *>(cell);
if (imp->m_marked) {
imp->m_marked = false;
- } else if (currentThreadIsMainThread || imp->m_destructorIsThreadSafe) {
+ } else {
+ ASSERT(currentThreadIsMainThread || !imp->m_collectOnMainThreadOnly);
+ if (imp->m_collectOnMainThreadOnly)
+ --mainThreadOnlyObjectCount;
imp->~JSCell();
--usedCells;
--numLiveObjects;
while (cell < heap.usedOversizeCells) {
JSCell *imp = (JSCell *)heap.oversizeCells[cell];
- if (!imp->m_marked && (currentThreadIsMainThread || imp->m_destructorIsThreadSafe)) {
+ if (imp->m_marked) {
+ imp->m_marked = false;
+ cell++;
+ } else {
+ ASSERT(currentThreadIsMainThread || !imp->m_collectOnMainThreadOnly);
+ if (imp->m_collectOnMainThreadOnly)
+ --mainThreadOnlyObjectCount;
imp->~JSCell();
#if DEBUG_COLLECTOR
heap.oversizeCells[cell]->u.freeCell.zeroIfFree = 0;
heap.numOversizeCells = heap.numOversizeCells / GROWTH_FACTOR;
heap.oversizeCells = (CollectorCell **)fastRealloc(heap.oversizeCells, heap.numOversizeCells * sizeof(CollectorCell *));
}
- } else {
- imp->m_marked = false;
- cell++;
}
}
namespace KJS {
- /**
- * @short Garbage collector.
- */
class Collector {
- // disallow direct construction/destruction
- Collector();
public:
static void* allocate(size_t s);
static bool collect();
static void protect(JSValue*);
static void unprotect(JSValue*);
+
+ static void collectOnMainThreadOnly(JSValue*);
static size_t numInterpreters();
static size_t numProtectedObjects();
class Thread;
static void registerThread();
-
+
private:
+ Collector();
static void markProtectedObjects();
+ static void markMainThreadOnlyObjects();
static void markCurrentThreadConservatively();
static void markOtherThreadConservatively(Thread* thread);
static void markStackObjectsConservatively();
static void markStackObjectsConservatively(void* start, void* end);
+ static size_t mainThreadOnlyObjectCount;
static bool memoryFull;
};
DatePrototype *dateProto)
: InternalFunctionImp(funcProto)
{
- // ECMA 15.9.4.1 Date.prototype
- putDirect(prototypePropertyName, dateProto, DontEnum|DontDelete|ReadOnly);
-
- static const Identifier parsePropertyName("parse");
- putDirectFunction(new DateObjectFuncImp(exec, funcProto, DateObjectFuncImp::Parse, 1, parsePropertyName), DontEnum);
- static const Identifier UTCPropertyName("UTC");
- putDirectFunction(new DateObjectFuncImp(exec, funcProto, DateObjectFuncImp::UTC, 7, UTCPropertyName), DontEnum);
+ static const Identifier* parsePropertyName = new Identifier("parse");
+ static const Identifier* UTCPropertyName = new Identifier("UTC");
- // no. of arguments for constructor
+ putDirect(prototypePropertyName, dateProto, DontEnum|DontDelete|ReadOnly);
+ putDirectFunction(new DateObjectFuncImp(exec, funcProto, DateObjectFuncImp::Parse, 1, *parsePropertyName), DontEnum);
+ putDirectFunction(new DateObjectFuncImp(exec, funcProto, DateObjectFuncImp::UTC, 7, *UTCPropertyName), DontEnum);
putDirect(lengthPropertyName, 7, ReadOnly|DontDelete|DontEnum);
}
FunctionPrototype::FunctionPrototype(ExecState *exec)
{
+ static const Identifier* applyPropertyName = new Identifier("apply");
+ static const Identifier* callPropertyName = new Identifier("call");
+
putDirect(lengthPropertyName, jsNumber(0), DontDelete|ReadOnly|DontEnum);
putDirectFunction(new FunctionProtoFunc(exec, this, FunctionProtoFunc::ToString, 0, toStringPropertyName), DontEnum);
- static const Identifier applyPropertyName("apply");
- putDirectFunction(new FunctionProtoFunc(exec, this, FunctionProtoFunc::Apply, 2, applyPropertyName), DontEnum);
- static const Identifier callPropertyName("call");
- putDirectFunction(new FunctionProtoFunc(exec, this, FunctionProtoFunc::Call, 1, callPropertyName), DontEnum);
+ putDirectFunction(new FunctionProtoFunc(exec, this, FunctionProtoFunc::Apply, 2, *applyPropertyName), DontEnum);
+ putDirectFunction(new FunctionProtoFunc(exec, this, FunctionProtoFunc::Call, 1, *callPropertyName), DontEnum);
}
FunctionPrototype::~FunctionPrototype()
#include "identifier.h"
+#include "JSLock.h"
+#include <new> // for placement new
+#include <string.h> // for strlen
+#include <wtf/Assertions.h>
#include <wtf/FastMalloc.h>
#include <wtf/HashSet.h>
-#include <string.h> // for strlen
-#include <new> // for placement new
namespace WTF {
static inline IdentifierTable& identifierTable()
{
+ ASSERT(JSLock::lockCount() > 0);
+
if (!table)
table = new IdentifierTable;
return *table;
return m_UriErrorPrototype;
}
-void Interpreter::mark(bool)
+void Interpreter::mark()
{
if (m_context)
m_context->mark();
* Called during the mark phase of the garbage collector. Subclasses
* implementing custom mark methods must make sure to chain to this one.
*/
- virtual void mark(bool currentThreadIsMainThread);
+ virtual void mark();
#ifdef KJS_DEBUG_MEM
/**
static inline ListImp *allocateListImp()
{
+ ASSERT(JSLock::lockCount() > 0);
+
// Find a free one in the pool.
if (poolUsed < poolSize) {
ListImp *imp = poolFreeList ? poolFreeList : &pool[0];
}
void List::release()
-{
+{
+ ASSERT(JSLock::lockCount() > 0);
+
ListImp *imp = static_cast<ListImp *>(_impBase);
#if DUMP_STATISTICS
poolFreeList = imp;
poolUsed--;
} else {
- assert(imp->state == usedOnHeap);
+ ASSERT(imp->state == usedOnHeap);
HeapListImp *list = static_cast<HeapListImp *>(imp);
// unlink from heap list
void List::append(JSValue *v)
{
+ ASSERT(JSLock::lockCount() > 0);
+
ListImp *imp = static_cast<ListImp *>(_impBase);
int i = imp->size++;
return copy;
}
-const List &List::empty()
+const List& List::empty()
{
- static List emptyList;
- return emptyList;
+ static List* staticEmptyList = new List;
+ return *staticEmptyList;
}
List &List::operator=(const List &b)
*
* @param proto The prototype
*/
- JSObject(JSValue* proto, bool destructorIsThreadSafe = true);
+ JSObject(JSValue* proto);
/**
* Creates a new JSObject with a prototype of jsNull()
* (that is, the ECMAScript "null" value, not a null object pointer).
*/
- explicit JSObject(bool destructorIsThreadSafe = true);
+ JSObject();
virtual void mark();
virtual JSType type() const;
JSObject *throwError(ExecState *, ErrorType, const char *message);
JSObject *throwError(ExecState *, ErrorType);
-inline JSObject::JSObject(JSValue* proto, bool destructorIsThreadSafe)
- : JSCell(destructorIsThreadSafe)
- , _proto(proto)
+inline JSObject::JSObject(JSValue* proto)
+ : _proto(proto)
{
assert(proto);
}
-inline JSObject::JSObject(bool destructorIsThreadSafe)
- : JSCell(destructorIsThreadSafe)
- , _proto(jsNull())
+inline JSObject::JSObject()
+ : _proto(jsNull())
{
}
ObjectPrototype::ObjectPrototype(ExecState* exec, FunctionPrototype* funcProto)
: JSObject() // [[Prototype]] is null
{
+ static const Identifier* hasOwnPropertyPropertyName = new Identifier("hasOwnProperty");
+ static const Identifier* propertyIsEnumerablePropertyName = new Identifier("propertyIsEnumerable");
+ static const Identifier* isPrototypeOfPropertyName = new Identifier("isPrototypeOf");
+ static const Identifier* defineGetterPropertyName = new Identifier("__defineGetter__");
+ static const Identifier* defineSetterPropertyName = new Identifier("__defineSetter__");
+ static const Identifier* lookupGetterPropertyName = new Identifier("__lookupGetter__");
+ static const Identifier* lookupSetterPropertyName = new Identifier("__lookupSetter__");
+
putDirectFunction(new ObjectProtoFunc(exec, funcProto, ObjectProtoFunc::ToString, 0, toStringPropertyName), DontEnum);
putDirectFunction(new ObjectProtoFunc(exec, funcProto, ObjectProtoFunc::ToLocaleString, 0, toLocaleStringPropertyName), DontEnum);
putDirectFunction(new ObjectProtoFunc(exec, funcProto, ObjectProtoFunc::ValueOf, 0, valueOfPropertyName), DontEnum);
- static Identifier hasOwnPropertyPropertyName("hasOwnProperty");
- static Identifier propertyIsEnumerablePropertyName("propertyIsEnumerable");
- static Identifier isPrototypeOfPropertyName("isPrototypeOf");
- putDirectFunction(new ObjectProtoFunc(exec, funcProto, ObjectProtoFunc::HasOwnProperty, 1, hasOwnPropertyPropertyName), DontEnum);
- putDirectFunction(new ObjectProtoFunc(exec, funcProto, ObjectProtoFunc::PropertyIsEnumerable, 1, propertyIsEnumerablePropertyName), DontEnum);
- putDirectFunction(new ObjectProtoFunc(exec, funcProto, ObjectProtoFunc::IsPrototypeOf, 1, isPrototypeOfPropertyName), DontEnum);
+ putDirectFunction(new ObjectProtoFunc(exec, funcProto, ObjectProtoFunc::HasOwnProperty, 1, *hasOwnPropertyPropertyName), DontEnum);
+ putDirectFunction(new ObjectProtoFunc(exec, funcProto, ObjectProtoFunc::PropertyIsEnumerable, 1, *propertyIsEnumerablePropertyName), DontEnum);
+ putDirectFunction(new ObjectProtoFunc(exec, funcProto, ObjectProtoFunc::IsPrototypeOf, 1, *isPrototypeOfPropertyName), DontEnum);
// Mozilla extensions
- static const Identifier defineGetterPropertyName("__defineGetter__");
- static const Identifier defineSetterPropertyName("__defineSetter__");
- static const Identifier lookupGetterPropertyName("__lookupGetter__");
- static const Identifier lookupSetterPropertyName("__lookupSetter__");
- putDirectFunction(new ObjectProtoFunc(exec, funcProto, ObjectProtoFunc::DefineGetter, 2, defineGetterPropertyName), DontEnum);
- putDirectFunction(new ObjectProtoFunc(exec, funcProto, ObjectProtoFunc::DefineSetter, 2, defineSetterPropertyName), DontEnum);
- putDirectFunction(new ObjectProtoFunc(exec, funcProto, ObjectProtoFunc::LookupGetter, 1, lookupGetterPropertyName), DontEnum);
- putDirectFunction(new ObjectProtoFunc(exec, funcProto, ObjectProtoFunc::LookupSetter, 1, lookupSetterPropertyName), DontEnum);
+ putDirectFunction(new ObjectProtoFunc(exec, funcProto, ObjectProtoFunc::DefineGetter, 2, *defineGetterPropertyName), DontEnum);
+ putDirectFunction(new ObjectProtoFunc(exec, funcProto, ObjectProtoFunc::DefineSetter, 2, *defineSetterPropertyName), DontEnum);
+ putDirectFunction(new ObjectProtoFunc(exec, funcProto, ObjectProtoFunc::LookupGetter, 1, *lookupGetterPropertyName), DontEnum);
+ putDirectFunction(new ObjectProtoFunc(exec, funcProto, ObjectProtoFunc::LookupSetter, 1, *lookupSetterPropertyName), DontEnum);
}
FunctionPrototype *funcProto)
: JSObject(objProto)
{
- // The constructor will be added later in RegExpObject's constructor (?)
+ static const Identifier* execPropertyName = new Identifier("exec");
+ static const Identifier* testPropertyName = new Identifier("test");
- static const Identifier execPropertyName("exec");
- static const Identifier testPropertyName("test");
- putDirectFunction(new RegExpProtoFunc(exec, funcProto, RegExpProtoFunc::Exec, 0, execPropertyName), DontEnum);
- putDirectFunction(new RegExpProtoFunc(exec, funcProto, RegExpProtoFunc::Test, 0, testPropertyName), DontEnum);
+ putDirectFunction(new RegExpProtoFunc(exec, funcProto, RegExpProtoFunc::Exec, 0, *execPropertyName), DontEnum);
+ putDirectFunction(new RegExpProtoFunc(exec, funcProto, RegExpProtoFunc::Test, 0, *testPropertyName), DontEnum);
putDirectFunction(new RegExpProtoFunc(exec, funcProto, RegExpProtoFunc::ToString, 0, toStringPropertyName), DontEnum);
}
#include "config.h"
#include "ustring.h"
+#include "JSLock.h"
+#include "dtoa.h"
+#include "identifier.h"
+#include "operations.h"
#include <assert.h>
-#include <stdlib.h>
-#include <stdio.h>
#include <ctype.h>
+#include <float.h>
+#include <math.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <wtf/Vector.h>
+
#if HAVE(STRING_H)
#include <string.h>
#endif
#include <strings.h>
#endif
-#include "dtoa.h"
-#include "identifier.h"
-#include "operations.h"
-#include <float.h>
-#include <math.h>
-#include <wtf/Vector.h>
-
using std::max;
using std::min;
UChar& UCharReference::ref() const
{
+ ASSERT(JSLock::lockCount() > 0);
+
if (offset < str->rep()->len)
return *(str->rep()->data() + offset);
else {
PassRefPtr<UString::Rep> UString::Rep::createCopying(const UChar *d, int l)
{
+ ASSERT(JSLock::lockCount() > 0);
+
int sizeInBytes = l * sizeof(UChar);
UChar *copyD = static_cast<UChar *>(fastMalloc(sizeInBytes));
memcpy(copyD, d, sizeInBytes);
PassRefPtr<UString::Rep> UString::Rep::create(UChar *d, int l)
{
+ ASSERT(JSLock::lockCount() > 0);
+
Rep *r = new Rep;
r->offset = 0;
r->len = l;
PassRefPtr<UString::Rep> UString::Rep::create(PassRefPtr<Rep> base, int offset, int length)
{
- assert(base);
+ ASSERT(JSLock::lockCount() > 0);
+ ASSERT(base);
int baseOffset = base->offset;
void UString::Rep::destroy()
{
+ ASSERT(JSLock::lockCount() > 0);
+
if (isIdentifier)
Identifier::remove(this);
if (baseString) {
}
}
-const UString &UString::null()
+const UString& UString::null()
{
- static UString n;
- return n;
+ static UString* n = new UString;
+ return *n;
}
UString UString::from(int i)
#ifndef _KJS_USTRING_H_
#define _KJS_USTRING_H_
+#include "JSLock.h"
+#include <stdint.h>
+#include <wtf/Assertions.h>
#include <wtf/FastMalloc.h>
-#include <wtf/RefPtr.h>
#include <wtf/PassRefPtr.h>
-
-#include <stdint.h>
+#include <wtf/RefPtr.h>
/* On ARM some versions of GCC don't pack structures by default so sizeof(UChar)
will end up being != 2 which causes crashes since the code depends on that. */
static unsigned computeHash(const UChar *, int length);
static unsigned computeHash(const char *);
- Rep* ref() { ++rc; return this; }
- void deref() { if (--rc == 0) destroy(); }
+ Rep* ref() { ASSERT(JSLock::lockCount() > 0); ++rc; return this; }
+ void deref() { ASSERT(JSLock::lockCount() > 0); if (--rc == 0) destroy(); }
// unshared data
int offset;
friend class JSObject;
friend class GetterSetterImp;
private:
- explicit JSCell(bool destructorIsThreadSafe = true);
+ JSCell();
virtual ~JSCell();
public:
// Querying the type.
bool marked() const;
private:
- bool m_destructorIsThreadSafe : 1;
+ bool m_collectOnMainThreadOnly : 1;
bool m_marked : 1;
};
{
}
-inline JSCell::JSCell(bool destructorIsThreadSafe)
- : m_destructorIsThreadSafe(destructorIsThreadSafe)
+inline JSCell::JSCell()
+ : m_collectOnMainThreadOnly(false)
, m_marked(false)
{
}
+2007-03-06 Geoffrey Garen <ggaren@apple.com>
+
+ Reviewed by Maciej Stachowiak.
+
+ Fixed all known crashers exposed by run-webkit-tests --threaded while using
+ a PAC file (for maximum carnage). See JavaScriptCore ChangeLog for
+ more details.
+
+ * JSBase.cpp:
+ (JSBase::Release): Lock when deleting, because we may be deleting an object
+ (like a JSRun) that holds thread-unsafe data.
+
+ * JSUtils.cpp:
+ (CFStringToUString): Don't lock, because our caller locks. Also, locking
+ inside a function that returns thread-unsafe data by copy will only mask
+ threading problems.
+
+ * JavaScriptGlue.cpp:
+ (JSRunEvaluate): Added missing JSLock.
+ (JSRunCheckSyntax): Converted to JSLock.
+ * JavaScriptGlue.xcodeproj/project.pbxproj:
+
2007-02-22 Geoffrey Garen <ggaren@apple.com>
Reviewed by Darin Adler.
{
if (--fRetainCount == 0)
{
+ JSLock lock;
delete this;
}
}
UString CFStringToUString(CFStringRef inCFString)
{
- JSLock lock;
-
UString result;
if (inCFString) {
CFIndex len = CFStringGetLength(inCFString);
JSRun* ptr = (JSRun*)ref;
if (ptr)
{
+ JSLock lock;
Completion completion = ptr->Evaluate();
if (completion.isValueCompletion())
{
JSRun* ptr = (JSRun*)ref;
if (ptr)
{
- JSLockInterpreter();
+ JSLock lock;
result = ptr->CheckSyntax();
- JSUnlockInterpreter();
}
return result;
}
0867D690FE84028FC02AAC07 /* Project object */ = {
isa = PBXProject;
buildConfigurationList = 14AC662B08CE7791006915A8 /* Build configuration list for PBXProject "JavaScriptGlue" */;
- compatibilityVersion = "Xcode 2.4";
hasScannedForEncodings = 1;
mainGroup = 0867D691FE84028FC02AAC07 /* JavaScriptGlue */;
productRefGroup = 034768DFFF38A50411DB9C8B /* Products */;
projectDirPath = "";
projectRoot = "";
- shouldCheckCompatibility = 1;
targets = (
DD66F3B908F73ED700C75FD7 /* JavaScriptGlue */,
1422E87609DE3BE800749B87 /* testjsglue */,
+2007-03-06 Geoffrey Garen <ggaren@apple.com>
+
+ Reviewed by Maciej Stachowiak.
+
+ Fixed all known crashers exposed by run-webkit-tests --threaded [*]. See
+ JavaScriptCore ChangeLog for more details.
+
+ * bindings/js/kjs_binding.cpp:
+ (KJS::domNodesPerDocument): Added thread safety ASSERT.
+ (KJS::ScriptInterpreter::mark): Removed obsolete logic for marking unsafe
+ objects when collecting on a secondary thread. The Collector takes care
+ of this now.
+
+ * bindings/js/kjs_binding.h:
+ (KJS::DOMObject::DOMObject): Used new API for specifying that WebCore
+ objects should be garbage collected on the main thread only.
+
+ * bindings/js/kjs_window.cpp:
+ (KJS::ScheduledAction::execute): Moved JSLock to cover implementedsCall() call,
+ which, for some subclasses, ends up allocating garbage collected objects.
+ (This fix was speculative. I didn't actually see a crash from this.)
+ (KJS::Window::timerFired): Added JSLock around ScheduleAction destruction,
+ since it destroys a KJS::List.
+
+ * bindings/objc/WebScriptObject.mm:
+ (-[WebScriptObject setException:]): Added JSLock. (This fix was speculative.
+ I didn't actually see a crash from this.)
+
+ * bridge/mac/WebCoreScriptDebugger.mm:
+ (-[WebCoreScriptCallFrame evaluateWebScript:]): Added JSLock. (This fix
+ was speculative. I didn't actually see a crash from this.)
+
+ * dom/Document.cpp:
+ (WebCore::Document::~Document): Added JSLock around modification to
+ domNodesPerDocument(), which can be accessed concurrently during garbage
+ collection.
+ * dom/Node.cpp:
+ (WebCore::Node::setDocument): ditto.
+
+ [*] fast/js/toString-stack-overflow.html is an exception. --threaded mode
+ crashes this test because it causes the garbage collector to run frequently,
+ and this test crashes if you happen to garbage collect while it's running.
+ This is a known issue with stack overflow during the mark phase. It's
+ not related to threading.
+
2007-03-06 Mark Rowe <mrowe@apple.com>
Reviewed by Sam Weinig.
static NodePerDocMap& domNodesPerDocument()
{
+ // domNodesPerDocument() callers must synchronize using the JSLock because
+ // domNodesPerDocument() is called during garbage collection, which can happen
+ // on a secondary thread.
+ ASSERT(JSLock::lockCount());
+
static NodePerDocMap staticDOMNodesPerDocument;
return staticDOMNodesPerDocument;
}
}
}
-void ScriptInterpreter::mark(bool currentThreadIsMainThread)
-{
- if (!currentThreadIsMainThread) {
- // On alternate threads, DOMObjects remain in the cache because they're not collected.
- // So, they need an opportunity to mark their children.
- DOMObjectMap::iterator objectEnd = domObjects().end();
- for (DOMObjectMap::iterator objectIt = domObjects().begin(); objectIt != objectEnd; ++objectIt) {
- DOMObject* object = objectIt->second;
- if (!object->marked())
- object->mark();
- }
- }
-
- Interpreter::mark(currentThreadIsMainThread);
-}
-
ExecState* ScriptInterpreter::globalExec()
{
// we need to make sure that any script execution happening in this
*/
class DOMObject : public JSObject {
protected:
- // DOMObject Destruction is not thread-safe because JS DOM objects
- // wrap unsafe WebCore DOM data structures
- DOMObject() : JSObject(false) {}
+ DOMObject()
+ {
+ // DOMObject destruction is not thread-safe because DOMObjects wrap
+ // unsafe WebCore DOM data structures.
+ Collector::collectOnMainThreadOnly(this);
+ }
#ifndef NDEBUG
virtual ~DOMObject();
#endif
*/
bool wasRunByUserGesture() const;
- virtual void mark(bool currentThreadIsMainThread);
virtual ExecState* globalExec();
WebCore::Event* getCurrentEvent() const { return m_currentEvent; }
public:
DOMWindowTimer(int timeoutId, int nestingLevel, Window* o, ScheduledAction* a)
: m_timeoutId(timeoutId), m_nestingLevel(nestingLevel), m_object(o), m_action(a) { }
- virtual ~DOMWindowTimer() { delete m_action; }
+ virtual ~DOMWindowTimer()
+ {
+ JSLock lock;
+ delete m_action;
+ }
int timeoutId() const { return m_timeoutId; }
interpreter->setProcessingTimerCallback(true);
if (JSValue* func = m_func.get()) {
+ JSLock lock;
if (func->isObject() && static_cast<JSObject*>(func)->implementsCall()) {
ExecState* exec = interpreter->globalExec();
ASSERT(window == interpreter->globalObject());
- JSLock lock;
interpreter->startTimeoutCheck();
static_cast<JSObject*>(func)->call(exec, window, m_args);
interpreter->stopTimeoutCheck();
m_timeouts.remove(timer->timeoutId());
delete timer;
action->execute(this);
+
+ JSLock lock;
delete action;
}
if (![self _rootObject])
return;
+ JSLock lock;
+
if ([self _rootObject]->interpreter()->context()) {
ExecState *exec = [self _rootObject]->interpreter()->context()->execState();
- (id)evaluateWebScript:(NSString *)script
{
+ JSLock lock;
+
UString code = String(script);
ExecState *state = _state;
// evaluate
JSValue *result;
if (eval) {
- JSLock lock;
List args;
args.append(jsString(code));
result = eval->call(state, NULL, args);
#endif
XMLHttpRequest::detachRequests(this);
- KJS::ScriptInterpreter::forgetAllDOMNodesForDocument(this);
+ {
+ KJS::JSLock lock;
+ KJS::ScriptInterpreter::forgetAllDOMNodesForDocument(this);
+ }
if (m_docChanged && changedDocuments)
changedDocuments->remove(this);
if (inDocument() || m_document == doc)
return;
- KJS::ScriptInterpreter::updateDOMNodeDocument(this, m_document.get(), doc);
+ {
+ KJS::JSLock lock;
+ KJS::ScriptInterpreter::updateDOMNodeDocument(this, m_document.get(), doc);
+ }
m_document = doc;
}
git://git.webkit.org / WebKit-https.git / commitdiff