Reviewed by Oliver Hunt.
Fixed <rdar://problem/
4681051> Installer crashes in KJS::Collector::
markOtherThreadConservatively(KJS::Collector::Thread*) trying to install
iLife 06 using Rosetta on an Intel Machine
The problem was that our thread-specific data destructor would modify the
list of active JavaScript threads without holding the JSLock, corrupting
the list. Corruption was especially likely if one JavaScript thread exited
while another was starting up.
* JavaScriptCore.exp:
* kjs/JSLock.cpp: Don't conflate locking the JSLock with registering a
thread, since the thread-specific data destructor needs to lock
without registering a thread. Instead, treat thread registration as a
part of the convenience of the JSLock object, and whittle down JSLock::lock()
to just the bits that actually do the locking.
(KJS::JSLock::lock):
(KJS::JSLock::registerThread):
* kjs/JSLock.h: Updated comments to mention the new behavior above, and
other recent changes.
(KJS::JSLock::JSLock):
* kjs/collector.cpp:
(KJS::destroyRegisteredThread): Lock here.
(KJS::Collector::registerThread): To match, assert that we're locked here.
JavaScriptGlue:
Reviewed by Oliver Hunt.
Updated in light of fix for <rdar://problem/
4681051> Installer crashes
in KJS::Collector::markOtherThreadConservatively(KJS::Collector::Thread*)
trying to install iLife 06 using Rosetta on an Intel Machine
* JavaScriptGlue.cpp:
(JSLockInterpreter): Ensure backwards compatibility by calling
registerThread() when explicitly taking the JSLock. (This doesn't happen
automatically anymore.) I doubt this actally matters, but in JavaScriptGlue
territory, that kind of thinking will get you killed.
WebKitTools:
Reviewed by Oliver Hunt.
Beefed up --threaded mode in light of <rdar://problem/
4681051> Installer
crashes in KJS::Collector::markOtherThreadConservatively(KJS::Collector::Thread*)
trying to install iLife 06 using Rosetta on an Intel Machine
--threaded mode now runs a bunch of different JavaScript threads, randomly
killing and respawning them. This was sufficient for reproducing the
bug on my MacBook Pro.
* DumpRenderTree/DumpRenderTree.m:
(javaScriptThreads):
(runJavaScriptThread):
(startJavaScriptThreads):
(stopJavaScriptThreads):
(dumpRenderTree):
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@20115
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2007-03-11 Geoffrey Garen <ggaren@apple.com>
+
+ Reviewed by Oliver Hunt.
+
+ Fixed <rdar://problem/4681051> Installer crashes in KJS::Collector::
+ markOtherThreadConservatively(KJS::Collector::Thread*) trying to install
+ iLife 06 using Rosetta on an Intel Machine
+
+ The problem was that our thread-specific data destructor would modify the
+ list of active JavaScript threads without holding the JSLock, corrupting
+ the list. Corruption was especially likely if one JavaScript thread exited
+ while another was starting up.
+
+ * JavaScriptCore.exp:
+ * kjs/JSLock.cpp: Don't conflate locking the JSLock with registering a
+ thread, since the thread-specific data destructor needs to lock
+ without registering a thread. Instead, treat thread registration as a
+ part of the convenience of the JSLock object, and whittle down JSLock::lock()
+ to just the bits that actually do the locking.
+ (KJS::JSLock::lock):
+ (KJS::JSLock::registerThread):
+ * kjs/JSLock.h: Updated comments to mention the new behavior above, and
+ other recent changes.
+ (KJS::JSLock::JSLock):
+ * kjs/collector.cpp:
+ (KJS::destroyRegisteredThread): Lock here.
+ (KJS::Collector::registerThread): To match, assert that we're locked here.
+
2007-03-10 Geoffrey Garen <ggaren@apple.com>
Reviewed by Darin Adler.
__ZN3KJS6JSCellnwEm
__ZN3KJS6JSLock12DropAllLocksC1Ev
__ZN3KJS6JSLock12DropAllLocksD1Ev
+__ZN3KJS6JSLock14registerThreadEv
__ZN3KJS6JSLock4lockEv
__ZN3KJS6JSLock6unlockEv
__ZN3KJS6JSLock9lockCountEv
pthread_setspecific(didLockJSMutex, &didLockJSMutex);
}
++JSLockCount;
-
- Collector::registerThread();
}
void JSLock::unlock()
}
}
+void JSLock::registerThread()
+{
+ Collector::registerThread();
+}
+
JSLock::DropAllLocks::DropAllLocks()
: m_lockCount(0)
{
{
}
+void JSLock::registerThread()
+{
+}
+
JSLock::DropAllLocks::DropAllLocks()
{
}
// To make it safe to use JavaScript on multiple threads, it is
// important to lock before doing anything that allocates a
- // garbage-collected object or which may affect other shared state
- // such as the protect count hash table. The simplest way to do
- // this is by having a local JSLock object for the scope
- // where the lock must be held. The lock is recursive so nesting
- // is ok.
+ // JavaScript data structure or that interacts with shared state
+ // such as the protect count hash table. The simplest way to lock
+ // is to create a local JSLock object in the scope where the lock
+ // must be held. The lock is recursive so nesting is ok. The JSLock
+ // object also acts as a convenience short-hand for running important
+ // initialization routines.
// To avoid deadlock, sometimes it is necessary to temporarily
// release the lock. Since it is recursive you actually have to
class JSLock : Noncopyable {
public:
- JSLock()
+ JSLock()
{
lock();
+ registerThread();
}
~JSLock()
static void unlock();
static int lockCount();
+ static void registerThread();
+
class DropAllLocks : Noncopyable {
public:
DropAllLocks();
{
Collector::Thread *thread = (Collector::Thread *)data;
+ // Can't use JSLock convenience object here because we don't want to re-register
+ // an exiting thread.
+ JSLock::lock();
+
if (registeredThreads == thread) {
registeredThreads = registeredThreads->next;
} else {
Collector::Thread *last = registeredThreads;
- for (Collector::Thread *t = registeredThreads->next; t != NULL; t = t->next) {
+ Collector::Thread *t;
+ for (t = registeredThreads->next; t != NULL; t = t->next) {
if (t == thread) {
last->next = t->next;
break;
}
last = t;
}
+ ASSERT(t); // If t is NULL, we never found ourselves in the list.
}
+ JSLock::unlock();
+
delete thread;
}
void Collector::registerThread()
{
+ ASSERT(JSLock::lockCount() > 0);
+
pthread_once(®isteredThreadKeyOnce, initializeRegisteredThreadKey);
if (!pthread_getspecific(registeredThreadKey)) {
+2007-03-12 Geoffrey Garen <ggaren@apple.com>
+
+ Reviewed by Oliver Hunt.
+
+ Updated in light of fix for <rdar://problem/4681051> Installer crashes
+ in KJS::Collector::markOtherThreadConservatively(KJS::Collector::Thread*)
+ trying to install iLife 06 using Rosetta on an Intel Machine
+
+ * JavaScriptGlue.cpp:
+ (JSLockInterpreter): Ensure backwards compatibility by calling
+ registerThread() when explicitly taking the JSLock. (This doesn't happen
+ automatically anymore.) I doubt this actally matters, but in JavaScriptGlue
+ territory, that kind of thinking will get you killed.
+
2007-03-06 Geoffrey Garen <ggaren@apple.com>
Reviewed by Maciej Stachowiak.
void JSLockInterpreter()
{
JSLock::lock();
+ JSLock::registerThread();
}
+2007-03-11 Geoffrey Garen <ggaren@apple.com>
+
+ Reviewed by Oliver Hunt.
+
+ Beefed up --threaded mode in light of <rdar://problem/4681051> Installer
+ crashes in KJS::Collector::markOtherThreadConservatively(KJS::Collector::Thread*)
+ trying to install iLife 06 using Rosetta on an Intel Machine
+
+ --threaded mode now runs a bunch of different JavaScript threads, randomly
+ killing and respawning them. This was sufficient for reproducing the
+ bug on my MacBook Pro.
+
+ * DumpRenderTree/DumpRenderTree.m:
+ (javaScriptThreads):
+ (runJavaScriptThread):
+ (startJavaScriptThreads):
+ (stopJavaScriptThreads):
+ (dumpRenderTree):
+
2007-03-11 Krzysztof Kowalczyk <kkowalczyk@gmail.com>
Reviewed by Brady Eidson.
const unsigned maxViewHeight = 600;
const unsigned maxViewWidth = 800;
-// Loops forever, running a script
+static pthread_mutex_t javaScriptThreadsMutex = PTHREAD_MUTEX_INITIALIZER;
+static BOOL javaScriptThreadsShouldTerminate;
+
+static const int javaScriptThreadsCount = 4;
+static CFMutableDictionaryRef javaScriptThreads()
+{
+ assert(pthread_mutex_trylock(&javaScriptThreadsMutex) == EBUSY);
+ static CFMutableDictionaryRef staticJavaScriptThreads;
+ if (!staticJavaScriptThreads)
+ staticJavaScriptThreads = CFDictionaryCreateMutable(kCFAllocatorDefault, 0, NULL, NULL);
+ return staticJavaScriptThreads;
+}
+
+// Loops forever, running a script and randomly respawning, until
+// javaScriptThreadsShouldTerminate becomes true.
void* runJavaScriptThread(void* arg)
{
- char* script =
+ const char* const script =
" \
var array = []; \
- for (var i = 0; i < 1000; i++) { \
+ for (var i = 0; i < 10; i++) { \
array.push(String(i)); \
} \
";
JSGarbageCollect(ctx);
- pthread_testcancel(); // Allow thread termination
+ pthread_mutex_lock(&javaScriptThreadsMutex);
+
+ // Check for cancellation.
+ if (javaScriptThreadsShouldTerminate) {
+ pthread_mutex_unlock(&javaScriptThreadsMutex);
+ return 0;
+ }
+
+ // Respawn probabilistically.
+ if (random() % 5 == 0) {
+ pthread_t pthread;
+ pthread_create(&pthread, NULL, &runJavaScriptThread, NULL);
+ pthread_detach(pthread);
+
+ CFDictionaryRemoveValue(javaScriptThreads(), pthread_self());
+ CFDictionaryAddValue(javaScriptThreads(), pthread, NULL);
+
+ pthread_mutex_unlock(&javaScriptThreadsMutex);
+ return 0;
+ }
+
+ pthread_mutex_unlock(&javaScriptThreadsMutex);
}
}
-static pthread_t javaScriptThread;
-
-static void startJavaScriptThread(void)
+static void startJavaScriptThreads(void)
{
- assert(!javaScriptThread);
- pthread_create(&javaScriptThread, NULL, runJavaScriptThread, NULL);
+ pthread_mutex_lock(&javaScriptThreadsMutex);
+
+ for (int i = 0; i < javaScriptThreadsCount; i++) {
+ pthread_t pthread;
+ pthread_create(&pthread, NULL, &runJavaScriptThread, NULL);
+ pthread_detach(pthread);
+ CFDictionaryAddValue(javaScriptThreads(), pthread, NULL);
+ }
+
+ pthread_mutex_unlock(&javaScriptThreadsMutex);
}
-static void stopJavaScriptThread(void)
+static void stopJavaScriptThreads(void)
{
- assert(javaScriptThread);
- pthread_cancel(javaScriptThread);
- javaScriptThread = NULL;
+ pthread_mutex_lock(&javaScriptThreadsMutex);
+
+ javaScriptThreadsShouldTerminate = YES;
+
+ const pthread_t pthreads[javaScriptThreadsCount];
+ assert(CFDictionaryGetCount(javaScriptThreads()) == javaScriptThreadsCount);
+ CFDictionaryGetKeysAndValues(javaScriptThreads(), (const void**)pthreads, NULL);
+
+ pthread_mutex_unlock(&javaScriptThreadsMutex);
+
+ for (int i = 0; i < javaScriptThreadsCount; i++) {
+ pthread_t pthread = pthreads[i];
+ pthread_join(pthread, NULL);
+ }
}
static BOOL shouldIgnoreWebCoreNodeLeaks(CFStringRef URLString)
[webView cacheDisplayInRect:[webView bounds] toBitmapImageRep:imageRep];
if (threaded)
- startJavaScriptThread();
+ startJavaScriptThreads();
if (argc == optind+1 && strcmp(argv[optind], "-") == 0) {
char filenameBuffer[2048];
}
if (threaded)
- stopJavaScriptThread();
+ stopJavaScriptThreads();
[workQueue release];
git://git.webkit.org / WebKit-https.git / commitdiff