Skip to content
  • ggaren's avatar
    JavaScriptCore: · ff59958f
    ggaren authored
            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: http://svn.webkit.org/repository/webkit/trunk@20004 268f45cc-cd09-0410-ab3c-d52691b4dbfc
    ff59958f