Changeset 10563 in webkit for trunk/JavaScriptCore/kjs/internal.cpp

Timestamp:

Sep 18, 2005, 11:57:28 PM (20 years ago)

Author:

mjs

Message:

Reviewed by Geoff.

• fixed <rdar://problem/4214783> REGRESSION: kjs_fast_malloc crash due to lack of locking on multiple threads (seen selecting volumes in the installer)

Make sure to lock using the InterpreterLock class in all places that need it
(including anything that uses the collector, the parser, the protect count hash table,
and anything that allocates via fast_malloc).

Also added assertions to ensure that the locking rules are followed for the relevant
resources.

• Makefile.am:
• bindings/NP_jsobject.cpp: (identifierFromNPIdentifier): (_NPN_Invoke): (_NPN_Evaluate): (_NPN_GetProperty): (_NPN_SetProperty): (_NPN_RemoveProperty): (_NPN_HasProperty): (_NPN_HasMethod): (_NPN_SetException):
• bindings/jni/jni_jsobject.cpp: (JSObject::call): (JSObject::eval): (JSObject::getMember): (JSObject::setMember): (JSObject::removeMember): (JSObject::getSlot): (JSObject::setSlot): (JSObject::toString): (JSObject::convertJObjectToValue):
• bindings/objc/WebScriptObject.mm: (-[WebScriptObject callWebScriptMethod:withArguments:]): (-[WebScriptObject evaluateWebScript:]): (-[WebScriptObject setValue:forKey:]): (-[WebScriptObject valueForKey:]): (-[WebScriptObject removeWebScriptKey:]): (-[WebScriptObject stringRepresentation]): (-[WebScriptObject webScriptValueAtIndex:]): (-[WebScriptObject setWebScriptValueAtIndex:value:]): (+[WebScriptObject _convertValueToObjcValue:KJS::originExecutionContext:Bindings::executionContext:Bindings::]):
• bindings/runtime.cpp: (Instance::createRuntimeObject):
• bindings/runtime_root.h:
• bindings/testbindings.cpp: (main):
• bindings/testbindings.mm: (main):
• kjs/fast_malloc.cpp: (KJS::kjs_fast_malloc): (KJS::kjs_fast_calloc): (KJS::kjs_fast_free): (KJS::kjs_fast_realloc):
• kjs/fast_malloc.h:
• kjs/identifier.h:
• kjs/internal.cpp: (InterpreterImp::InterpreterImp): (InterpreterImp::clear): (InterpreterImp::mark): (InterpreterImp::checkSyntax): (InterpreterImp::evaluate):
• kjs/internal.h: (KJS::InterpreterImp::globalObject):
• kjs/interpreter.cpp: (Interpreter::evaluate):
• kjs/interpreter.h: (KJS::InterpreterLock::InterpreterLock): (KJS::InterpreterLock::~InterpreterLock):
• kjs/nodes.h:
• kjs/protect.h: (KJS::ProtectedValue::ProtectedValue): (KJS::ProtectedValue::~ProtectedValue): (KJS::ProtectedValue::operator=): (KJS::ProtectedObject::ProtectedObject): (KJS::ProtectedObject::~ProtectedObject): (KJS::ProtectedObject::operator=): (KJS::ProtectedReference::ProtectedReference): (KJS::ProtectedReference::~ProtectedReference): (KJS::ProtectedReference::operator=):
• kjs/protected_object.h:
• kjs/protected_values.cpp: (KJS::ProtectedValues::getProtectCount): (KJS::ProtectedValues::increaseProtectCount): (KJS::ProtectedValues::decreaseProtectCount):
• kjs/string_object.cpp: (StringObjectImp::StringObjectImp):
• kjs/testkjs.cpp: (main):

File:

• trunk/JavaScriptCore/kjs/internal.cpp (modified) (12 diffs)

trunk/JavaScriptCore/kjs/internal.cpp

@@ -1 @@
-// -*- c-basic-offset: 2 -*-
 /*
  *  This file is part of the KDE libraries
@@ -452 +451 @@
   // add this interpreter to the global chain
   // as a root set for garbage collection
-  lockInterpreter();
+  InterpreterLock lock;
+
   m_interpreter = interp;
   if (s_hook) {
@@ -475 +475 @@
 
   recursion = 0;
-  unlockInterpreter();
 }
 
@@ -623 +622 @@
   //fprintf(stderr,"InterpreterImp::clear\n");
   // remove from global chain (see init())
-#if APPLE_CHANGES
-  lockInterpreter();
-#endif
+  InterpreterLock lock;
+
   next->prev = prev;
   prev->next = next;
@@ -636 +634 @@
   }
   InterpreterMap::removeInterpreterForGlobalObject(global);
-
-#if APPLE_CHANGES
-  unlockInterpreter();
-#endif
 }
 
@@ -649 +643 @@
   if (_context)
     _context->mark();
+  if (global)
+      global->mark();
+  if (globExec._exception)
+      globExec._exception->mark();
 }
 
 bool InterpreterImp::checkSyntax(const UString &code)
 {
+  InterpreterLock lock;
+
   // Parser::parse() returns 0 in a syntax error occurs, so we just check for that
   SharedPtr<ProgramNode> progNode = Parser::parse(UString(), 0, code.data(),code.size(),0,0,0);
@@ -660 +660 @@
 Completion InterpreterImp::evaluate(const UString &code, ValueImp *thisV, const UString &sourceURL, int startingLineNumber)
 {
-#if APPLE_CHANGES
-  lockInterpreter();
-#endif
+  InterpreterLock lock;
+
   // prevent against infinite recursion
   if (recursion >= 20) {
 #if APPLE_CHANGES
     Completion result = Completion(Throw, Error::create(&globExec, GeneralError, "Recursion too deep"));
-    unlockInterpreter();
     return result;
 #else
@@ -673 +671 @@
 #endif
   }
-  
+
   // parse the source code
   int sid;
@@ -684 +682 @@
     bool cont = dbg->sourceParsed(&globExec, sid, sourceURL, code, errLine);
     if (!cont)
-#if APPLE_CHANGES
-      {
-        unlockInterpreter();
-        return Completion(Break);
-      }
-#else
       return Completion(Break);
-#endif
   }
   
@@ -697 +688 @@
   if (!progNode) {
     ObjectImp *err = Error::create(&globExec, SyntaxError, errMsg, errLine, sid, &sourceURL);
-#if APPLE_CHANGES
-    unlockInterpreter();
-#endif
     return Completion(Throw,err);
   }
@@ -728 +716 @@
     // execute the code
     ContextImp ctx(globalObj, this, thisObj);
-    ExecState newExec(m_interpreter,&ctx);
+    ExecState newExec(m_interpreter, &ctx);
     progNode->processVarDecls(&newExec);
     res = progNode->execute(&newExec);
@@ -735 +723 @@
   recursion--;
 
-#if APPLE_CHANGES
-  unlockInterpreter();
-#endif
   return res;
 }