[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373
ggaren at apple.com
ggaren at apple.com
Thu Apr 8 01:14:23 UTC 2010
The following commit has been merged in the webkit-1.2 branch:
commit 49db29a484e35187336dfbe54bce400b650adc96
Author: ggaren at apple.com <ggaren at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Tue Jan 19 08:39:04 2010 +0000
JavaScriptCore: REGRESSION (52082): Crash on worker thread when reloading http://radnan.public.iastate.edu/procedural/
https://bugs.webkit.org/show_bug.cgi?id=33826
Reviewed by Oliver Hunt.
This bug was caused by a GC-protected object being destroyed early by
Heap::destroy. Clients of the GC protect APIs (reasonably) expect pointers
to GC-protected memory to be valid.
The solution is to do two passes of tear-down in Heap::destroy. The first
pass tears down all unprotected objects. The second pass ASSERTs that all
previously protected objects are now unprotected, and then tears down
all perviously protected objects. These two passes simulate the two passes
that would have been required to free a protected object during normal GC.
* API/JSContextRef.cpp: Removed some ASSERTs that have moved into Heap.
* runtime/Collector.cpp:
(JSC::Heap::destroy): Moved ASSERTs to here.
(JSC::Heap::freeBlock): Tidied up the use of didShrink by moving its
setter to the function that does the shrinking.
(JSC::Heap::freeBlocks): Implemented above algorithm.
(JSC::Heap::shrinkBlocks): Tidied up the use of didShrink.
WebCore: REGRESSION (52082): Crash on worker thread when reloading http://radnan.public.iastate.edu/procedural/
https://bugs.webkit.org/show_bug.cgi?id=33826
Reviewed by Oliver Hunt.
Test: fast/workers/worker-gc2.html
* bindings/js/WorkerScriptController.cpp:
(WebCore::WorkerScriptController::~WorkerScriptController): Removed some
ASSERTs that have moved to JavaScriptCore.
LayoutTests: REGRESSION (52082): Crash on worker thread when reloading http://radnan.public.iastate.edu/procedural/
https://bugs.webkit.org/show_bug.cgi?id=33826
Reviewed by Oliver Hunt.
Added a test for this edge case.
* fast/workers/resources/worker-gc2.js: Added.
(Dummy):
* fast/workers/worker-gc2.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@53460 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/JavaScriptCore/API/JSContextRef.cpp b/JavaScriptCore/API/JSContextRef.cpp
index 35eac04..6bdc3c8 100644
--- a/JavaScriptCore/API/JSContextRef.cpp
+++ b/JavaScriptCore/API/JSContextRef.cpp
@@ -127,8 +127,6 @@ void JSGlobalContextRelease(JSGlobalContextRef ctx)
JSGlobalData& globalData = exec->globalData();
if (globalData.refCount() == 2) { // One reference is held by JSGlobalObject, another added by JSGlobalContextRetain().
// The last reference was released, this is our last chance to collect.
- ASSERT(!globalData.heap.protectedObjectCount());
- ASSERT(!globalData.heap.isBusy());
globalData.heap.destroy();
} else
globalData.heap.collectAllGarbage();
diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index 65ad189..e76a545 100644
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,29 @@
+2010-01-19 Geoffrey Garen <ggaren at apple.com>
+
+ Reviewed by Oliver Hunt.
+
+ REGRESSION (52082): Crash on worker thread when reloading http://radnan.public.iastate.edu/procedural/
+ https://bugs.webkit.org/show_bug.cgi?id=33826
+
+ This bug was caused by a GC-protected object being destroyed early by
+ Heap::destroy. Clients of the GC protect APIs (reasonably) expect pointers
+ to GC-protected memory to be valid.
+
+ The solution is to do two passes of tear-down in Heap::destroy. The first
+ pass tears down all unprotected objects. The second pass ASSERTs that all
+ previously protected objects are now unprotected, and then tears down
+ all perviously protected objects. These two passes simulate the two passes
+ that would have been required to free a protected object during normal GC.
+
+ * API/JSContextRef.cpp: Removed some ASSERTs that have moved into Heap.
+
+ * runtime/Collector.cpp:
+ (JSC::Heap::destroy): Moved ASSERTs to here.
+ (JSC::Heap::freeBlock): Tidied up the use of didShrink by moving its
+ setter to the function that does the shrinking.
+ (JSC::Heap::freeBlocks): Implemented above algorithm.
+ (JSC::Heap::shrinkBlocks): Tidied up the use of didShrink.
+
2010-01-19 Gavin Barraclough <barraclough at apple.com>
Reviewed by NOBODY (build fix).
diff --git a/JavaScriptCore/jsc.cpp b/JavaScriptCore/jsc.cpp
index 864b2ce..252fb96 100644
--- a/JavaScriptCore/jsc.cpp
+++ b/JavaScriptCore/jsc.cpp
@@ -292,6 +292,11 @@ JSValue JSC_HOST_CALL functionReadline(ExecState* exec, JSObject*, JSValue, cons
JSValue JSC_HOST_CALL functionQuit(ExecState* exec, JSObject*, JSValue, const ArgList&)
{
+ // Technically, destroying the heap in the middle of JS execution is a no-no,
+ // but we want to maintain compatibility with the Mozilla test suite, so
+ // we pretend that execution has terminated to avoid ASSERTs, then tear down the heap.
+ exec->globalData().dynamicGlobalObject = 0;
+
cleanupGlobalData(&exec->globalData());
exit(EXIT_SUCCESS);
diff --git a/JavaScriptCore/runtime/Collector.cpp b/JavaScriptCore/runtime/Collector.cpp
index f15cb2c..e02c289 100644
--- a/JavaScriptCore/runtime/Collector.cpp
+++ b/JavaScriptCore/runtime/Collector.cpp
@@ -186,6 +186,9 @@ void Heap::destroy()
if (!m_globalData)
return;
+ ASSERT(!m_globalData->dynamicGlobalObject);
+ ASSERT(!isBusy());
+
// 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);
@@ -290,6 +293,8 @@ NEVER_INLINE CollectorBlock* Heap::allocateBlock()
NEVER_INLINE void Heap::freeBlock(size_t block)
{
+ m_heap.didShrink = true;
+
ObjectIterator it(m_heap, block);
ObjectIterator end(m_heap, block + 1);
for ( ; it != end; ++it)
@@ -329,9 +334,29 @@ NEVER_INLINE void Heap::freeBlockPtr(CollectorBlock* block)
void Heap::freeBlocks()
{
- while (m_heap.usedBlocks)
- freeBlock(0);
+ ProtectCountSet protectedValuesCopy = m_protectedValues;
+
+ clearMarkBits();
+ markProtectedObjects(m_globalData->markStack);
+
+ m_heap.nextCell = 0;
+ m_heap.nextBlock = 0;
+ DeadObjectIterator it(m_heap, m_heap.nextBlock, m_heap.nextCell);
+ DeadObjectIterator end(m_heap, m_heap.usedBlocks);
+ for ( ; it != end; ++it)
+ (*it)->~JSCell();
+
+ ASSERT(!protectedObjectCount());
+
+ ProtectCountSet::iterator protectedValuesEnd = protectedValuesCopy.end();
+ for (ProtectCountSet::iterator protectedValuesIt = protectedValuesCopy.begin(); protectedValuesIt != protectedValuesEnd; ++protectedValuesIt)
+ protectedValuesIt->first->~JSCell();
+
+ for (size_t block = 0; block < m_heap.usedBlocks; ++block)
+ freeBlockPtr(m_heap.blocks[block]);
+
fastFree(m_heap.blocks);
+
memset(&m_heap, 0, sizeof(CollectorHeap));
}
@@ -440,7 +465,6 @@ void Heap::shrinkBlocks(size_t neededBlocks)
for (size_t i = 0; i != m_heap.usedBlocks && m_heap.usedBlocks != neededBlocks; ) {
if (m_heap.blocks[i]->marked.isEmpty()) {
freeBlock(i);
- m_heap.didShrink = true;
} else
++i;
}
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index e941e00..5bcce6b 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2010-01-19 Geoffrey Garen <ggaren at apple.com>
+
+ Reviewed by Oliver Hunt.
+
+ REGRESSION (52082): Crash on worker thread when reloading http://radnan.public.iastate.edu/procedural/
+ https://bugs.webkit.org/show_bug.cgi?id=33826
+
+ Added a test for this edge case.
+
+ * fast/workers/resources/worker-gc2.js: Added.
+ (Dummy):
+ * fast/workers/worker-gc2.html: Added.
+
2010-01-19 Gavin Barraclough <barraclough at apple.com>
Reviewed by NOBODY (build fix).
diff --git a/LayoutTests/fast/workers/resources/worker-gc2.js b/LayoutTests/fast/workers/resources/worker-gc2.js
new file mode 100644
index 0000000..2f8f84e
--- /dev/null
+++ b/LayoutTests/fast/workers/resources/worker-gc2.js
@@ -0,0 +1,25 @@
+function Dummy()
+{
+ this.x = 1;
+ this.y = 1;
+}
+
+(function () {
+ var d = new Dummy;
+ var a = [];
+
+ // Create an iterator at the beginning of the heap.
+ for (var p in d) {
+ a[a.length] = p;
+ }
+
+ // Fill the middle of the heap with blocks of garbage.
+ for (var i = 0; i < 64 * 1024; ++i)
+ a[a.length] = new Object;
+
+ // Create an object sharing the structure pointed to by the above iterator late in the heap.
+ new Dummy;
+
+ postMessage('done');
+ close();
+})();
diff --git a/LayoutTests/fast/workers/worker-gc2.html b/LayoutTests/fast/workers/worker-gc2.html
new file mode 100644
index 0000000..a30150e
--- /dev/null
+++ b/LayoutTests/fast/workers/worker-gc2.html
@@ -0,0 +1,34 @@
+<p>This page tests for a GC crash when tearing down a worker. If the tests passes,
+you'll see a PASS message below.</p>
+
+<pre id="console"></pre>
+
+<script>
+function $(id)
+{
+ return document.getElementById(id);
+}
+
+function log(s)
+{
+ $("console").appendChild(document.createTextNode(s + "\n"));
+}
+
+(function () {
+ if (window.layoutTestController) {
+ layoutTestController.dumpAsText();
+ layoutTestController.waitUntilDone();
+ }
+
+ var worker = new Worker("resources/worker-gc2.js");
+ worker.onmessage = function () {
+ log("PASS: You didn't crash.");
+
+ // Try to wait for the worker to finish closing.
+ setTimeout(function () {
+ if (window.layoutTestController)
+ layoutTestController.notifyDone();
+ }, 0);
+ }
+})();
+</script>
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 453802e..48c5943 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,16 @@
+2010-01-19 Geoffrey Garen <ggaren at apple.com>
+
+ Reviewed by Oliver Hunt.
+
+ REGRESSION (52082): Crash on worker thread when reloading http://radnan.public.iastate.edu/procedural/
+ https://bugs.webkit.org/show_bug.cgi?id=33826
+
+ Test: fast/workers/worker-gc2.html
+
+ * bindings/js/WorkerScriptController.cpp:
+ (WebCore::WorkerScriptController::~WorkerScriptController): Removed some
+ ASSERTs that have moved to JavaScriptCore.
+
2010-01-19 Gavin Barraclough <barraclough at apple.com>
Reviewed by NOBODY (build fix).
diff --git a/WebCore/bindings/js/WorkerScriptController.cpp b/WebCore/bindings/js/WorkerScriptController.cpp
index 5e27ef7..adcc089 100644
--- a/WebCore/bindings/js/WorkerScriptController.cpp
+++ b/WebCore/bindings/js/WorkerScriptController.cpp
@@ -58,9 +58,6 @@ WorkerScriptController::WorkerScriptController(WorkerContext* workerContext)
WorkerScriptController::~WorkerScriptController()
{
m_workerContextWrapper = 0; // Unprotect the global object.
-
- ASSERT(!m_globalData->heap.protectedObjectCount());
- ASSERT(!m_globalData->heap.isBusy());
m_globalData->heap.destroy();
}
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list