<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" /><style type="text/css"><!--
#msg dl { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fc0 solid; padding: 6px; }
#msg ul, pre { overflow: auto; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<title>[20019] trunk</title>
</head>
<body>
<div id="msg">
<dl>
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/20019">20019</a></dd>
<dt>Author</dt> <dd>ggaren</dd>
<dt>Date</dt> <dd>2007-03-07 07:37:06 -0800 (Wed, 07 Mar 2007)</dd>
</dl>
<h3>Log Message</h3>
<pre>JavaScriptCore:
Reviewed by Maciej.
http://bugs.webkit.org/show_bug.cgi?id=12997
Wrap pthread-specific assertion in #if USE(MULTIPLE_THREADS).
* kjs/collector.cpp:
(KJS::Collector::markMainThreadOnlyObjects):
WebCore:
Reviewed by Maciej Stachowiak.
Fixed <rdar://problem/4576242> | http://bugs.webkit.org/show_bug.cgi?id=12586
PAC file: malloc deadlock sometimes causes a hang @ www.apple.com/pro/profiles/ (12586)
No test because this is very difficult to repro, and the new ASSERTs in
JavaScriptCore catch the underlying cause while running normal layout tests.
This is a modified version of r14752 on the branch.
The fix is to use a bit inside each node, instead of a hash table, to track
which node subtrees are in the process of being marked. This avoids a call
to malloc inside mark().
* bindings/js/kjs_binding.cpp:
(KJS::domObjects):
(KJS::domNodesPerDocument):
* bindings/js/kjs_dom.cpp:
(KJS::DOMNode::mark):
* dom/Node.cpp:
(WebCore::Node::Node):
* dom/Node.h:</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkJavaScriptCoreChangeLog">trunk/JavaScriptCore/ChangeLog</a></li>
<li><a href="#trunkJavaScriptCorekjscollectorcpp">trunk/JavaScriptCore/kjs/collector.cpp</a></li>
<li><a href="#trunkJavaScriptCorewtfFastMalloccpp">trunk/JavaScriptCore/wtf/FastMalloc.cpp</a></li>
<li><a href="#trunkJavaScriptCorewtfFastMalloch">trunk/JavaScriptCore/wtf/FastMalloc.h</a></li>
<li><a href="#trunkWebCoreChangeLog">trunk/WebCore/ChangeLog</a></li>
<li><a href="#trunkWebCorebindingsjskjs_bindingcpp">trunk/WebCore/bindings/js/kjs_binding.cpp</a></li>
<li><a href="#trunkWebCorebindingsjskjs_domcpp">trunk/WebCore/bindings/js/kjs_dom.cpp</a></li>
<li><a href="#trunkWebCoredomNodecpp">trunk/WebCore/dom/Node.cpp</a></li>
<li><a href="#trunkWebCoredomNodeh">trunk/WebCore/dom/Node.h</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkJavaScriptCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/JavaScriptCore/ChangeLog (20018 => 20019)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/JavaScriptCore/ChangeLog 2007-03-07 15:01:10 UTC (rev 20018)
+++ trunk/JavaScriptCore/ChangeLog 2007-03-07 15:37:06 UTC (rev 20019)
</span><span class="lines">@@ -13,6 +13,31 @@
</span><span class="cx">
</span><span class="cx"> Reviewed by Maciej Stachowiak.
</span><span class="cx">
</span><ins>+ Fixed <rdar://problem/4576242> | http://bugs.webkit.org/show_bug.cgi?id=12586
+ PAC file: malloc deadlock sometimes causes a hang @ www.apple.com/pro/profiles/ (12586)
+
+ This is a modified version of r14752 on the branch.
+
+ These changes just add debugging functionality. They ASSERT that we don't
+ malloc during the mark phase of a garbage collection, which can cause a
+ deadlock.
+
+ * kjs/collector.cpp:
+ (KJS::Collector::collect):
+ * wtf/FastMalloc.cpp:
+ (WTF::fastMallocForbid):
+ (WTF::fastMallocAllow):
+ (WTF::fastMalloc):
+ (WTF::fastCalloc):
+ (WTF::fastFree):
+ (WTF::fastRealloc):
+ (WTF::do_malloc):
+ * wtf/FastMalloc.h:
+
+2007-03-06 Geoffrey Garen <ggaren@apple.com>
+
+ Reviewed by Maciej Stachowiak.
+
</ins><span class="cx"> Fixed all known crashers exposed by run-webkit-tests --threaded. This covers:
</span><span class="cx">
</span><span class="cx"> <rdar://problem/4565394> | http://bugs.webkit.org/show_bug.cgi?id=12585
</span></span></pre></div>
<a id="trunkJavaScriptCorekjscollectorcpp"></a>
<div class="modfile"><h4>Modified: trunk/JavaScriptCore/kjs/collector.cpp (20018 => 20019)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/JavaScriptCore/kjs/collector.cpp 2007-03-07 15:01:10 UTC (rev 20018)
+++ trunk/JavaScriptCore/kjs/collector.cpp 2007-03-07 15:37:06 UTC (rev 20019)
</span><span class="lines">@@ -564,6 +564,15 @@
</span><span class="cx"> bool currentThreadIsMainThread = true;
</span><span class="cx"> #endif
</span><span class="cx">
</span><ins>+ // MARK: first mark all referenced objects recursively starting out from the set of root objects
+
+#ifndef NDEBUG
+ // Forbid malloc during the mark phase. Marking a thread suspends it, so
+ // a malloc inside mark() would risk a deadlock with a thread that had been
+ // suspended while holding the malloc lock.
+ fastMallocForbid();
+#endif
+
</ins><span class="cx"> if (Interpreter::s_hook) {
</span><span class="cx"> Interpreter* scr = Interpreter::s_hook;
</span><span class="cx"> do {
</span><span class="lines">@@ -572,14 +581,16 @@
</span><span class="cx"> } while (scr != Interpreter::s_hook);
</span><span class="cx"> }
</span><span class="cx">
</span><del>- // MARK: first mark all referenced objects recursively starting out from the set of root objects
-
</del><span class="cx"> markStackObjectsConservatively();
</span><span class="cx"> markProtectedObjects();
</span><span class="cx"> List::markProtectedLists();
</span><span class="cx"> if (!currentThreadIsMainThread)
</span><span class="cx"> markMainThreadOnlyObjects();
</span><span class="cx">
</span><ins>+#ifndef NDEBUG
+ fastMallocAllow();
+#endif
+
</ins><span class="cx"> // SWEEP: delete everything with a zero refcount (garbage) and unmark everything else
</span><span class="cx">
</span><span class="cx"> size_t emptyBlocks = 0;
</span></span></pre></div>
<a id="trunkJavaScriptCorewtfFastMalloccpp"></a>
<div class="modfile"><h4>Modified: trunk/JavaScriptCore/wtf/FastMalloc.cpp (20018 => 20019)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/JavaScriptCore/wtf/FastMalloc.cpp 2007-03-07 15:01:10 UTC (rev 20018)
+++ trunk/JavaScriptCore/wtf/FastMalloc.cpp 2007-03-07 15:37:06 UTC (rev 20019)
</span><span class="lines">@@ -65,6 +65,8 @@
</span><span class="cx"> #include "config.h"
</span><span class="cx"> #include "FastMalloc.h"
</span><span class="cx">
</span><ins>+#include "Assertions.h"
+
</ins><span class="cx"> #ifndef USE_SYSTEM_MALLOC
</span><span class="cx"> #ifndef NDEBUG
</span><span class="cx"> #define USE_SYSTEM_MALLOC 1
</span><span class="lines">@@ -73,6 +75,23 @@
</span><span class="cx"> #endif
</span><span class="cx"> #endif
</span><span class="cx">
</span><ins>+#ifndef NDEBUG
+namespace WTF {
+
+static bool isForbidden = false;
+void fastMallocForbid()
+{
+ isForbidden = true;
+}
+
+void fastMallocAllow()
+{
+ isForbidden = false;
+}
+
+} // namespace WTF
+#endif
+
</ins><span class="cx"> #if USE_SYSTEM_MALLOC
</span><span class="cx">
</span><span class="cx"> #include <stdlib.h>
</span><span class="lines">@@ -84,21 +103,25 @@
</span><span class="cx">
</span><span class="cx"> void *fastMalloc(size_t n)
</span><span class="cx"> {
</span><ins>+ ASSERT(!isForbidden);
</ins><span class="cx"> return malloc(n);
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void *fastCalloc(size_t n_elements, size_t element_size)
</span><span class="cx"> {
</span><ins>+ ASSERT(!isForbidden);
</ins><span class="cx"> return calloc(n_elements, element_size);
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void fastFree(void* p)
</span><span class="cx"> {
</span><ins>+ ASSERT(!isForbidden);
</ins><span class="cx"> free(p);
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void *fastRealloc(void* p, size_t n)
</span><span class="cx"> {
</span><ins>+ ASSERT(!isForbidden);
</ins><span class="cx"> return realloc(p, n);
</span><span class="cx"> }
</span><span class="cx">
</span><span class="lines">@@ -1880,6 +1903,7 @@
</span><span class="cx">
</span><span class="cx"> #ifdef WTF_CHANGES
</span><span class="cx"> ASSERT(isMultiThreaded || pthread_main_np());
</span><ins>+ ASSERT(!isForbidden);
</ins><span class="cx"> #endif
</span><span class="cx">
</span><span class="cx"> #ifndef WTF_CHANGES
</span></span></pre></div>
<a id="trunkJavaScriptCorewtfFastMalloch"></a>
<div class="modfile"><h4>Modified: trunk/JavaScriptCore/wtf/FastMalloc.h (20018 => 20019)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/JavaScriptCore/wtf/FastMalloc.h 2007-03-07 15:01:10 UTC (rev 20018)
+++ trunk/JavaScriptCore/wtf/FastMalloc.h 2007-03-07 15:37:06 UTC (rev 20019)
</span><span class="lines">@@ -34,6 +34,11 @@
</span><span class="cx"> void fastFree(void* p);
</span><span class="cx"> void *fastRealloc(void* p, size_t n);
</span><span class="cx">
</span><ins>+#ifndef NDEBUG
+ void fastMallocForbid();
+ void fastMallocAllow();
+#endif
+
</ins><span class="cx"> } // namespace WTF
</span><span class="cx">
</span><span class="cx"> using WTF::fastMalloc;
</span><span class="lines">@@ -41,6 +46,11 @@
</span><span class="cx"> using WTF::fastRealloc;
</span><span class="cx"> using WTF::fastFree;
</span><span class="cx">
</span><ins>+#ifndef NDEBUG
+using WTF::fastMallocForbid;
+using WTF::fastMallocAllow;
+#endif
+
</ins><span class="cx"> #if PLATFORM(GCC) && PLATFORM(DARWIN)
</span><span class="cx"> #define WTF_PRIVATE_INLINE __private_extern__ inline __attribute__((always_inline))
</span><span class="cx"> #elif PLATFORM(GCC)
</span></span></pre></div>
<a id="trunkWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/ChangeLog (20018 => 20019)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/ChangeLog 2007-03-07 15:01:10 UTC (rev 20018)
+++ trunk/WebCore/ChangeLog 2007-03-07 15:37:06 UTC (rev 20019)
</span><span class="lines">@@ -1,3 +1,28 @@
</span><ins>+2007-03-06 Geoffrey Garen <ggaren@apple.com>
+
+ Reviewed by Maciej Stachowiak.
+
+ Fixed <rdar://problem/4576242> | http://bugs.webkit.org/show_bug.cgi?id=12586
+ PAC file: malloc deadlock sometimes causes a hang @ www.apple.com/pro/profiles/ (12586)
+
+ No test because this is very difficult to repro, and the new ASSERTs in
+ JavaScriptCore catch the underlying cause while running normal layout tests.
+
+ This is a modified version of r14752 on the branch.
+
+ The fix is to use a bit inside each node, instead of a hash table, to track
+ which node subtrees are in the process of being marked. This avoids a call
+ to malloc inside mark().
+
+ * bindings/js/kjs_binding.cpp:
+ (KJS::domObjects):
+ (KJS::domNodesPerDocument):
+ * bindings/js/kjs_dom.cpp:
+ (KJS::DOMNode::mark):
+ * dom/Node.cpp:
+ (WebCore::Node::Node):
+ * dom/Node.h:
+
</ins><span class="cx"> 2007-03-06 David Hyatt <hyatt@apple.com>
</span><span class="cx">
</span><span class="cx"> This patch reworks the WebCore memory cache to significantly reduce the amount of memory consumed by
</span></span></pre></div>
<a id="trunkWebCorebindingsjskjs_bindingcpp"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/bindings/js/kjs_binding.cpp (20018 => 20019)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/bindings/js/kjs_binding.cpp 2007-03-07 15:01:10 UTC (rev 20018)
+++ trunk/WebCore/bindings/js/kjs_binding.cpp 2007-03-07 15:37:06 UTC (rev 20019)
</span><span class="lines">@@ -113,6 +113,7 @@
</span><span class="cx">
</span><span class="cx"> static DOMObjectMap& domObjects()
</span><span class="cx"> {
</span><ins>+ // Don't use malloc here. Calling malloc from a mark function can deadlock.
</ins><span class="cx"> static DOMObjectMap staticDOMObjects;
</span><span class="cx"> return staticDOMObjects;
</span><span class="cx"> }
</span><span class="lines">@@ -120,10 +121,11 @@
</span><span class="cx"> static NodePerDocMap& domNodesPerDocument()
</span><span class="cx"> {
</span><span class="cx"> // domNodesPerDocument() callers must synchronize using the JSLock because
</span><del>- // domNodesPerDocument() is called during garbage collection, which can happen
</del><ins>+ // domNodesPerDocument() is called from a mark function, which can run
</ins><span class="cx"> // on a secondary thread.
</span><span class="cx"> ASSERT(JSLock::lockCount());
</span><span class="cx">
</span><ins>+ // Don't use malloc here. Calling malloc from a mark function can deadlock.
</ins><span class="cx"> static NodePerDocMap staticDOMNodesPerDocument;
</span><span class="cx"> return staticDOMNodesPerDocument;
</span><span class="cx"> }
</span></span></pre></div>
<a id="trunkWebCorebindingsjskjs_domcpp"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/bindings/js/kjs_dom.cpp (20018 => 20019)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/bindings/js/kjs_dom.cpp 2007-03-07 15:01:10 UTC (rev 20018)
+++ trunk/WebCore/bindings/js/kjs_dom.cpp 2007-03-07 15:37:06 UTC (rev 20019)
</span><span class="lines">@@ -139,17 +139,15 @@
</span><span class="cx"> root = current;
</span><span class="cx"> }
</span><span class="cx">
</span><del>- static HashSet<Node*> markingRoots;
-
</del><span class="cx"> // If we're already marking this tree, then we can simply mark this wrapper
</span><span class="cx"> // by calling the base class; our caller is iterating the tree.
</span><del>- if (markingRoots.contains(root)) {
</del><ins>+ if (root->m_inSubtreeMark) {
</ins><span class="cx"> DOMObject::mark();
</span><span class="cx"> return;
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> // Mark the whole tree; use the global set of roots to avoid reentering.
</span><del>- markingRoots.add(root);
</del><ins>+ root->m_inSubtreeMark = true;
</ins><span class="cx"> for (Node* nodeToMark = root; nodeToMark; nodeToMark = nodeToMark->traverseNextNode()) {
</span><span class="cx"> DOMNode *wrapper = ScriptInterpreter::getDOMNodeForDocument(m_impl->document(), nodeToMark);
</span><span class="cx"> if (wrapper) {
</span><span class="lines">@@ -165,7 +163,7 @@
</span><span class="cx"> mark();
</span><span class="cx"> }
</span><span class="cx"> }
</span><del>- markingRoots.remove(root);
</del><ins>+ root->m_inSubtreeMark = false;
</ins><span class="cx">
</span><span class="cx"> // Double check that we actually ended up marked. This assert caught problems in the past.
</span><span class="cx"> assert(marked());
</span></span></pre></div>
<a id="trunkWebCoredomNodecpp"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/dom/Node.cpp (20018 => 20019)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/dom/Node.cpp 2007-03-07 15:01:10 UTC (rev 20018)
+++ trunk/WebCore/dom/Node.cpp 2007-03-07 15:37:06 UTC (rev 20019)
</span><span class="lines">@@ -154,7 +154,8 @@
</span><span class="cx"> m_hovered(false),
</span><span class="cx"> m_inActiveChain(false),
</span><span class="cx"> m_implicit(false),
</span><del>- m_inDetach(false)
</del><ins>+ m_inDetach(false),
+ m_inSubtreeMark(false)
</ins><span class="cx"> {
</span><span class="cx"> #ifndef NDEBUG
</span><span class="cx"> if (shouldIgnoreLeaks)
</span></span></pre></div>
<a id="trunkWebCoredomNodeh"></a>
<div class="modfile"><h4>Modified: trunk/WebCore/dom/Node.h (20018 => 20019)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/WebCore/dom/Node.h 2007-03-07 15:01:10 UTC (rev 20018)
+++ trunk/WebCore/dom/Node.h 2007-03-07 15:37:06 UTC (rev 20019)
</span><span class="lines">@@ -459,6 +459,7 @@
</span><span class="cx">
</span><span class="cx"> short m_tabIndex;
</span><span class="cx">
</span><ins>+ // 16 bit fields exactly -- adding another field would increase the size of all Nodes
</ins><span class="cx"> bool m_hasId : 1;
</span><span class="cx"> bool m_hasClass : 1;
</span><span class="cx"> bool m_hasStyle : 1;
</span><span class="lines">@@ -477,6 +478,9 @@
</span><span class="cx">
</span><span class="cx"> bool m_inDetach : 1;
</span><span class="cx">
</span><ins>+public:
+ bool m_inSubtreeMark : 1;
+
</ins><span class="cx"> private:
</span><span class="cx"> Element* ancestorElement() const;
</span><span class="cx"> };
</span></span></pre>
</div>
</div>
</body>
</html>