Changeset 20115 in webkit for trunk/JavaScriptCore/kjs/collector.cpp

Timestamp:

Mar 12, 2007, 8:09:37 AM (18 years ago)

Author:

ggaren

Message:

JavaScriptCore:

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):

File:

• trunk/JavaScriptCore/kjs/collector.cpp (modified) (3 diffs)

trunk/JavaScriptCore/kjs/collector.cpp

@@ -237 +237 @@
   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;
@@ -248 +253 @@
       last = t;
     }
-  }
+    ASSERT(t); // If t is NULL, we never found ourselves in the list.
+  }
+
+  JSLock::unlock();
 
   delete thread;
@@ -260 +268 @@
 void Collector::registerThread()
 {
+  ASSERT(JSLock::lockCount() > 0);
+  
   pthread_once(&registeredThreadKeyOnce, initializeRegisteredThreadKey);