Changeset 55256 in webkit
- Timestamp:
- Feb 25, 2010 2:15:26 PM (14 years ago)
- Location:
- trunk
- Files:
-
- 7 edited
Legend:
- Unmodified
- Added
- Removed
-
trunk/JavaScriptCore/ChangeLog
r55247 r55256 1 2010-02-25 Oliver Hunt <oliver@apple.com> 2 3 Reviewed by Maciej Stachowiak. 4 5 Race condition in JSPropertyNameIterator and Structure destruction 6 https://bugs.webkit.org/show_bug.cgi?id=35398 7 8 JSPropertyNameIterator and Structure have a cyclic dependency that they 9 manage by clearing the appropriate reference in each other during their 10 destruction. However if the Structure is destroyed while the 11 JSPropertyNameIterator is dead but not yet finalized the Structures 12 WeakGCPtr will return null, and so prevent Structure from clearing 13 the m_cachedStructure pointer of the iterator. When the iterator is 14 then finalised the m_cachedStructure is invalid, and the attempt to 15 clear the structures back reference fails. 16 17 To fix this we simply make JSPropertyNameIterator keep the Structure 18 alive, using the weak pointer to break the ref cycle. 19 20 * runtime/JSPropertyNameIterator.cpp: 21 (JSC::JSPropertyNameIterator::~JSPropertyNameIterator): 22 The iterator now keeps m_cachedStructure alive itself, so no longer needs 23 to check for it being cleared 24 * runtime/JSPropertyNameIterator.h: 25 (JSC::JSPropertyNameIterator::setCachedStructure): 26 Add an assertion to ensure correct usage 27 (JSC::JSPropertyNameIterator::cachedStructure): 28 Add .get() 29 * runtime/Structure.cpp: 30 (JSC::Structure::~Structure): 31 Add an assertion that our iterator isn't already dead, and remove 32 the now unnecessary attempt to clear the ref in the iterator 33 * runtime/WeakGCPtr.h: 34 (JSC::WeakGCPtr::hasDeadObject): 35 An assert-only function to allow us to assert correct behaviour 36 in the Structure destructor 37 1 38 2010-02-25 Jochen Eisinger <jochen@chromium.org> 2 39 -
trunk/JavaScriptCore/runtime/JSPropertyNameIterator.cpp
r54696 r55256 50 50 JSPropertyNameIterator::~JSPropertyNameIterator() 51 51 { 52 if (m_cachedStructure) 53 m_cachedStructure->clearEnumerationCache(this); 52 m_cachedStructure->clearEnumerationCache(this); 54 53 } 55 54 -
trunk/JavaScriptCore/runtime/JSPropertyNameIterator.h
r54696 r55256 68 68 size_t size() { return m_jsStringsSize; } 69 69 70 void setCachedStructure(Structure* structure) { m_cachedStructure = structure; } 71 Structure* cachedStructure() { return m_cachedStructure; } 70 void setCachedStructure(Structure* structure) 71 { 72 ASSERT(!m_cachedStructure); 73 ASSERT(structure); 74 m_cachedStructure = structure; 75 } 76 Structure* cachedStructure() { return m_cachedStructure.get(); } 72 77 73 78 void setCachedPrototypeChain(NonNullPassRefPtr<StructureChain> cachedPrototypeChain) { m_cachedPrototypeChain = cachedPrototypeChain; } … … 77 82 JSPropertyNameIterator(ExecState*, PropertyNameArrayData* propertyNameArrayData, size_t numCacheableSlot); 78 83 79 Structure*m_cachedStructure;84 RefPtr<Structure> m_cachedStructure; 80 85 RefPtr<StructureChain> m_cachedPrototypeChain; 81 86 uint32_t m_numCacheableSlots; -
trunk/JavaScriptCore/runtime/Structure.cpp
r54798 r55256 266 266 267 267 } 268 269 if (m_enumerationCache) 270 m_enumerationCache->setCachedStructure(0); 268 ASSERT(!m_enumerationCache.hasDeadObject()); 271 269 272 270 if (m_propertyTable) { -
trunk/JavaScriptCore/runtime/WeakGCPtr.h
r54428 r55256 66 66 WeakGCPtr& operator=(T*); 67 67 68 #if !ASSERT_DISABLED 69 bool hasDeadObject() const { return !!m_ptr; } 70 #endif 71 68 72 private: 69 73 void assign(T* ptr) -
trunk/LayoutTests/ChangeLog
r55251 r55256 1 2010-02-25 Oliver Hunt <oliver@apple.com> 2 3 Reviewed by Maciej Stachowiak. 4 5 Race condition in JSPropertyNameIterator and Structure destruction 6 https://bugs.webkit.org/show_bug.cgi?id=35398 7 8 Add test to ensure that this race condition doesn't occur. 9 10 * fast/js/script-tests/for-in-cached.js: 11 (cacheClearing): 12 1 13 2010-02-25 Alexey Proskuryakov <ap@apple.com> 2 14 -
trunk/LayoutTests/fast/js/script-tests/for-in-cached.js
r50323 r55256 65 65 shouldBe("forIn5({get foo() { return 'called getter'}, set foo() { }})", "['foo', 'called getter']"); 66 66 67 function cacheClearing() { 68 for(var j=0; j < 10; j++){ 69 var o = {a:1,b:2,c:3,d:4,e:5} 70 try {for (i in o) { delete o.a; o = null; throw "" };}finally{continue} 71 } 72 } 73 74 cacheClearing() 67 75 68 76 var successfullyParsed = true;
Note: See TracChangeset
for help on using the changeset viewer.