Changeset 115579 in webkit for trunk/Source/JavaScriptCore/heap/Heap.cpp

Timestamp:

Apr 28, 2012, 1:51:27 PM (13 years ago)

Author:

[email protected]

Message:

Clarified JSGlobalData (JavaScript VM) lifetime
​https://bugs.webkit.org/show_bug.cgi?id=85142

Reviewed by Anders Carlsson.

Source/JavaScriptCore:

This was so confusing that I didn't feel like I could reason about
memory lifetime in the heap without fixing it.

The rules are:

(1) JSGlobalData owns the virtual machine and all memory in it.

(2) Deleting a JSGlobalData frees the virtual machine and all memory
in it.

(Caveat emptor: if you delete the virtual machine while you're running
JIT code or accessing GC objects, you're gonna have a bad time.)

(I opted not to make arbitrary sub-objects keep the virtual machine
alive automatically because:

(a) doing that right would be complex and slow;

(b) in the case of an exiting thread or process, there's no
clear way to give the garbage collector a chance to try again
later;

(c) continuing to run the garbage collector after we've been
asked to shut down the virtual machine seems rude;

(d) we've never really supported that feature, anyway.)

(3) Normal ref-counting will do. No need to call a battery of
specialty functions to tear down a JSGlobalData. Its foibles
notwithstanding, C++ does in fact know how to execute destructors in
order.

• API/JSContextRef.cpp:

(JSGlobalContextCreate): Removed compatibility shim for older
operating systems because it's no longer used.

(JSGlobalContextRelease): Now that we can rely on JSGlobalData to "do
the right thing", this code is much simpler. We still have one special
case to notify the garbage collector if we're removing the last
reference to the global object, since this can improve memory behavior.

• heap/CopiedSpace.cpp:

(JSC::CopiedSpace::freeAllBlocks):

• heap/CopiedSpace.h:

(CopiedSpace): Renamed "destroy" => "freeAllBlocks" because true
destruction-time behaviors should be limited to our C++ destructor.

• heap/Heap.cpp:

(JSC::Heap::~Heap):
(JSC):
(JSC::Heap::lastChanceToFinalize):

• heap/Heap.h:

(Heap):
(JSC::Heap::heap): Renamed "destroy" => "lastChanceToFinalize" because
true destruction-time behaviors should be limited to our C++
destructor.

Reorganized the code, putting code that must run before any objects
get torn down into lastChanceToFinalize, and code that just tears down
objects into our destructor.

• heap/Local.h:

(JSC::LocalStack::LocalStack):
(JSC::LocalStack::push):
(LocalStack): See rule (2).

• jsc.cpp:

(functionQuit):
(main):
(printUsageStatement):
(parseArguments):
(jscmain):

• testRegExp.cpp:

(main):
(printUsageStatement):
(parseArguments):
(realMain): See rule (3).

I removed the feature of ensuring orderly tear-down when calling quit()
or running in --help mode because it didn't seem very useful and
making it work with Windows structured exception handling and
NO_RETURN didn't seem like a fun way to spend a Saturday.

• runtime/JSGlobalData.h:
• runtime/JSGlobalData.cpp:

(JSC::JSGlobalData::JSGlobalData): Moved heap to be the first data
member in JSGlobalData to ensure that it's destructed last, so other
objects that reference it destruct without crashing. This allowed me
to remove clearBuiltinStructures() altogether, and helped guarantee
rule (3).

(JSC::JSGlobalData::~JSGlobalData): Explicitly call
lastChanceToFinalize() at the head of our destructor to ensure that
all pending finalizers run while the virtual machine is still in a
valid state. Trying to resurrect (re-ref) the virtual machine at this
point is not valid, but all other operations are.

Changed a null to a 0xbbadbeef to clarify just how bad this beef is.

• runtime/JSGlobalObject.cpp:

(JSC::JSGlobalObject::init):

• runtime/JSGlobalObject.h:

(JSGlobalObject):
(JSC::JSGlobalObject::globalData): See rule (3).

Source/WebCore:

• bindings/js/WorkerScriptController.cpp:

(WebCore::WorkerScriptController::~WorkerScriptController): Slightly
simpler than before. We can't just rely on our default destructor
because we need to hold the JSLock when we tear down the VM.

• bridge/NP_jsobject.cpp:

(_NPN_InvokeDefault):
(_NPN_Invoke):
(_NPN_Evaluate):
(_NPN_Construct): Don't RefPtr<> the JSGlobalData because it makes it
seem like you know something the rest of our code doesn't know. The
plugin JSGlobalData is immortal, anyway.

I also removed some timeout checker related code because that feature
doesn't work anymore, so it was effectively dead code.

Source/WebKit/mac:

• Plugins/Hosted/NetscapePluginInstanceProxy.mm:

(WebKit::NetscapePluginInstanceProxy::invoke):
(WebKit::NetscapePluginInstanceProxy::invokeDefault):
(WebKit::NetscapePluginInstanceProxy::construct):

File:

• trunk/Source/JavaScriptCore/heap/Heap.cpp (modified) (3 diffs)

trunk/Source/JavaScriptCore/heap/Heap.cpp

@@ -342 +342 @@
 Heap::~Heap()
 {
-    // Destroy our block freeing thread.
+    delete m_markListSet;
+
+    m_objectSpace.shrink();
+    m_storageSpace.freeAllBlocks();
+    releaseFreeBlocks();
     {
         MutexLocker locker(m_freeBlockLock);
@@ -350 +354 @@
     waitForThreadCompletion(m_blockFreeingThread);
 
-    // The destroy function must already have been called, so assert this.
-    ASSERT(!m_globalData);
-}
-
-void Heap::destroy()
-{
-    JSLock lock(SilenceAssertionsOnly);
-
-    if (!m_globalData)
-        return;
-
+    ASSERT(!size());
+    ASSERT(!capacity());
+}
+
+// The JSGlobalData is being destroyed and the collector will never run again.
+// Run all pending finalizers now because we won't get another chance.
+void Heap::lastChanceToFinalize()
+{
     ASSERT(!m_globalData->dynamicGlobalObject);
     ASSERT(m_operationInProgress == NoOperation);
-    
-    // The global object is not GC protected at this point, so sweeping may delete it
-    // (and thus the global data) before other objects that may use the global data.
-    RefPtr<JSGlobalData> protect(m_globalData);
-
-#if ENABLE(JIT)
-    m_globalData->jitStubs->clearHostFunctionStubs();
-#endif
-
-    delete m_markListSet;
-    m_markListSet = 0;
-
+
+    // FIXME: Make this a release-mode crash once we're sure no one's doing this.
+    if (size_t size = m_protectedValues.size())
+        LOG_ERROR("JavaScriptCore heap deallocated while %ld values were still protected", size);
+
+    m_weakSet.finalizeAll();
     canonicalizeCellLivenessData();
     clearMarks();
-
-    m_weakSet.finalizeAll();
+    sweep();
     m_globalData->smallStrings.finalizeSmallStrings();
-    m_objectSpace.shrink();
-    m_storageSpace.destroy();
-    ASSERT(!size());
 
 #if ENABLE(SIMPLE_HEAP_PROFILING)
@@ -388 +379 @@
     m_destroyedTypeCounts.dump(WTF::dataFile(), "Destroyed Type Counts");
 #endif
-    
-    releaseFreeBlocks();
-
-    m_globalData = 0;
 }