[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:15 UTC 2010
The following commit has been merged in the webkit-1.2 branch:
commit ad2c3af8bb40788da9ba05cc7ec86601b7892dcc
Author: ggaren at apple.com <ggaren at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Tue Jan 19 07:00:38 2010 +0000
2010-01-18 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-18 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-18 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.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@53455 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 8231e6c..50329b4 100644
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,29 @@
+2010-01-18 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-18 Gavin Barraclough <barraclough at apple.com>
Reviewed by Oliver Hunt.
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 c8c28d3..f6dc479 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2010-01-18 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-18 Daniel Bates <dbates at webkit.org>
https://bugs.webkit.org/show_bug.cgi?id=33089
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 574d758..971ac4f 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,16 @@
+2010-01-18 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-18 Daniel Bates <dbates at webkit.org>
Reviewed by Darin Adler.
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