[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373
oliver at apple.com
oliver at apple.com
Thu Apr 8 01:59:30 UTC 2010
The following commit has been merged in the webkit-1.2 branch:
commit 43b309bf76ffca835f9300c058186573982014f1
Author: oliver at apple.com <oliver at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Thu Feb 25 22:15:26 2010 +0000
2010-02-25 Oliver Hunt <oliver at apple.com>
Reviewed by Maciej Stachowiak.
Race condition in JSPropertyNameIterator and Structure destruction
https://bugs.webkit.org/show_bug.cgi?id=35398
JSPropertyNameIterator and Structure have a cyclic dependency that they
manage by clearing the appropriate reference in each other during their
destruction. However if the Structure is destroyed while the
JSPropertyNameIterator is dead but not yet finalized the Structures
WeakGCPtr will return null, and so prevent Structure from clearing
the m_cachedStructure pointer of the iterator. When the iterator is
then finalised the m_cachedStructure is invalid, and the attempt to
clear the structures back reference fails.
To fix this we simply make JSPropertyNameIterator keep the Structure
alive, using the weak pointer to break the ref cycle.
* runtime/JSPropertyNameIterator.cpp:
(JSC::JSPropertyNameIterator::~JSPropertyNameIterator):
The iterator now keeps m_cachedStructure alive itself, so no longer needs
to check for it being cleared
* runtime/JSPropertyNameIterator.h:
(JSC::JSPropertyNameIterator::setCachedStructure):
Add an assertion to ensure correct usage
(JSC::JSPropertyNameIterator::cachedStructure):
Add .get()
* runtime/Structure.cpp:
(JSC::Structure::~Structure):
Add an assertion that our iterator isn't already dead, and remove
the now unnecessary attempt to clear the ref in the iterator
* runtime/WeakGCPtr.h:
(JSC::WeakGCPtr::hasDeadObject):
An assert-only function to allow us to assert correct behaviour
in the Structure destructor
2010-02-25 Oliver Hunt <oliver at apple.com>
Reviewed by Maciej Stachowiak.
Race condition in JSPropertyNameIterator and Structure destruction
https://bugs.webkit.org/show_bug.cgi?id=35398
Add test to ensure that this race condition doesn't occur.
* fast/js/script-tests/for-in-cached.js:
(cacheClearing):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@55256 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/JavaScriptCore/ChangeLog b/JavaScriptCore/ChangeLog
index 58eeee8..f855d5e 100644
--- a/JavaScriptCore/ChangeLog
+++ b/JavaScriptCore/ChangeLog
@@ -1,3 +1,40 @@
+2010-02-25 Oliver Hunt <oliver at apple.com>
+
+ Reviewed by Maciej Stachowiak.
+
+ Race condition in JSPropertyNameIterator and Structure destruction
+ https://bugs.webkit.org/show_bug.cgi?id=35398
+
+ JSPropertyNameIterator and Structure have a cyclic dependency that they
+ manage by clearing the appropriate reference in each other during their
+ destruction. However if the Structure is destroyed while the
+ JSPropertyNameIterator is dead but not yet finalized the Structures
+ WeakGCPtr will return null, and so prevent Structure from clearing
+ the m_cachedStructure pointer of the iterator. When the iterator is
+ then finalised the m_cachedStructure is invalid, and the attempt to
+ clear the structures back reference fails.
+
+ To fix this we simply make JSPropertyNameIterator keep the Structure
+ alive, using the weak pointer to break the ref cycle.
+
+ * runtime/JSPropertyNameIterator.cpp:
+ (JSC::JSPropertyNameIterator::~JSPropertyNameIterator):
+ The iterator now keeps m_cachedStructure alive itself, so no longer needs
+ to check for it being cleared
+ * runtime/JSPropertyNameIterator.h:
+ (JSC::JSPropertyNameIterator::setCachedStructure):
+ Add an assertion to ensure correct usage
+ (JSC::JSPropertyNameIterator::cachedStructure):
+ Add .get()
+ * runtime/Structure.cpp:
+ (JSC::Structure::~Structure):
+ Add an assertion that our iterator isn't already dead, and remove
+ the now unnecessary attempt to clear the ref in the iterator
+ * runtime/WeakGCPtr.h:
+ (JSC::WeakGCPtr::hasDeadObject):
+ An assert-only function to allow us to assert correct behaviour
+ in the Structure destructor
+
2010-02-25 Jochen Eisinger <jochen at chromium.org>
Reviewed by Jeremy Orlow.
diff --git a/JavaScriptCore/runtime/JSPropertyNameIterator.cpp b/JavaScriptCore/runtime/JSPropertyNameIterator.cpp
index a5d4da0..2e85a9b 100644
--- a/JavaScriptCore/runtime/JSPropertyNameIterator.cpp
+++ b/JavaScriptCore/runtime/JSPropertyNameIterator.cpp
@@ -49,8 +49,7 @@ inline JSPropertyNameIterator::JSPropertyNameIterator(ExecState* exec, PropertyN
JSPropertyNameIterator::~JSPropertyNameIterator()
{
- if (m_cachedStructure)
- m_cachedStructure->clearEnumerationCache(this);
+ m_cachedStructure->clearEnumerationCache(this);
}
JSPropertyNameIterator* JSPropertyNameIterator::create(ExecState* exec, JSObject* o)
diff --git a/JavaScriptCore/runtime/JSPropertyNameIterator.h b/JavaScriptCore/runtime/JSPropertyNameIterator.h
index 3f533a0..01700ac 100644
--- a/JavaScriptCore/runtime/JSPropertyNameIterator.h
+++ b/JavaScriptCore/runtime/JSPropertyNameIterator.h
@@ -67,8 +67,13 @@ namespace JSC {
JSValue get(ExecState*, JSObject*, size_t i);
size_t size() { return m_jsStringsSize; }
- void setCachedStructure(Structure* structure) { m_cachedStructure = structure; }
- Structure* cachedStructure() { return m_cachedStructure; }
+ void setCachedStructure(Structure* structure)
+ {
+ ASSERT(!m_cachedStructure);
+ ASSERT(structure);
+ m_cachedStructure = structure;
+ }
+ Structure* cachedStructure() { return m_cachedStructure.get(); }
void setCachedPrototypeChain(NonNullPassRefPtr<StructureChain> cachedPrototypeChain) { m_cachedPrototypeChain = cachedPrototypeChain; }
StructureChain* cachedPrototypeChain() { return m_cachedPrototypeChain.get(); }
@@ -76,7 +81,7 @@ namespace JSC {
private:
JSPropertyNameIterator(ExecState*, PropertyNameArrayData* propertyNameArrayData, size_t numCacheableSlot);
- Structure* m_cachedStructure;
+ RefPtr<Structure> m_cachedStructure;
RefPtr<StructureChain> m_cachedPrototypeChain;
uint32_t m_numCacheableSlots;
uint32_t m_jsStringsSize;
diff --git a/JavaScriptCore/runtime/Structure.cpp b/JavaScriptCore/runtime/Structure.cpp
index ebf8a4c..6f23c7d 100644
--- a/JavaScriptCore/runtime/Structure.cpp
+++ b/JavaScriptCore/runtime/Structure.cpp
@@ -265,9 +265,7 @@ Structure::~Structure()
m_previous->transitionTableRemove(make_pair(m_nameInPrevious.get(), m_attributesInPrevious), m_specificValueInPrevious);
}
-
- if (m_enumerationCache)
- m_enumerationCache->setCachedStructure(0);
+ ASSERT(!m_enumerationCache.hasDeadObject());
if (m_propertyTable) {
unsigned entryCount = m_propertyTable->keyCount + m_propertyTable->deletedSentinelCount;
diff --git a/JavaScriptCore/runtime/WeakGCPtr.h b/JavaScriptCore/runtime/WeakGCPtr.h
index 3ed4645..5f58374 100644
--- a/JavaScriptCore/runtime/WeakGCPtr.h
+++ b/JavaScriptCore/runtime/WeakGCPtr.h
@@ -65,6 +65,10 @@ public:
WeakGCPtr& operator=(T*);
+#if !ASSERT_DISABLED
+ bool hasDeadObject() const { return !!m_ptr; }
+#endif
+
private:
void assign(T* ptr)
{
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 3512d8e..ecfbf6a 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+2010-02-25 Oliver Hunt <oliver at apple.com>
+
+ Reviewed by Maciej Stachowiak.
+
+ Race condition in JSPropertyNameIterator and Structure destruction
+ https://bugs.webkit.org/show_bug.cgi?id=35398
+
+ Add test to ensure that this race condition doesn't occur.
+
+ * fast/js/script-tests/for-in-cached.js:
+ (cacheClearing):
+
2010-02-25 Alexey Proskuryakov <ap at apple.com>
Tiger build fix.
diff --git a/LayoutTests/fast/js/script-tests/for-in-cached.js b/LayoutTests/fast/js/script-tests/for-in-cached.js
index c86d62c..8a1a14b 100644
--- a/LayoutTests/fast/js/script-tests/for-in-cached.js
+++ b/LayoutTests/fast/js/script-tests/for-in-cached.js
@@ -64,5 +64,13 @@ shouldBe("forIn5({get foo() { return 'called getter'} })", "['foo', 'called gett
shouldBe("forIn5({set foo() { } })", "['foo', undefined]");
shouldBe("forIn5({get foo() { return 'called getter'}, set foo() { }})", "['foo', 'called getter']");
+function cacheClearing() {
+ for(var j=0; j < 10; j++){
+ var o = {a:1,b:2,c:3,d:4,e:5}
+ try {for (i in o) { delete o.a; o = null; throw "" };}finally{continue}
+ }
+}
+
+cacheClearing()
var successfullyParsed = true;
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list