[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc
antonm at chromium.org
antonm at chromium.org
Wed Dec 22 11:21:43 UTC 2010
The following commit has been merged in the debian/experimental branch:
commit 81a4e47205ad8cf61a4b379f9d86660b9773e448
Author: antonm at chromium.org <antonm at chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Tue Jul 20 17:20:49 2010 +0000
2010-07-20 Anton Muhin <antonm at chromium.org>
Reviewed by Adam Barth.
[v8] Allow handles to be disposed and WebKit objects to be dereferenced even if their map is already destroyed.
https://bugs.webkit.org/show_bug.cgi?id=42634
Currently DOMDataStore could be destroyed even if it has some mappings (it gets destroyed
when its isolated context gets GCed). However in this case, handles allocated for
such objects would never be disposed as we require presence of mapping from wrapped
WebKit object to handle being collected in the map and now map is gone. That leads to
zombie objects in both WebKit (wrapped WebKit object doesn't get dereferenced) and V8
(both handle and V8 wrapper object could not be destroyed).
See http://code.google.com/p/chromium/issues/detail?id=47125 for further discussion.
* bindings/v8/DOMData.h:
(WebCore::DOMData::handleWeakObject):
* bindings/v8/DOMDataStore.cpp:
(WebCore::DOMDataStore::weakNodeCallback):
* bindings/v8/V8DOMMap.h:
(WebCore::WeakReferenceMap::~WeakReferenceMap):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@63751 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 317dcfc..8571d09 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,26 @@
+2010-07-20 Anton Muhin <antonm at chromium.org>
+
+ Reviewed by Adam Barth.
+
+ [v8] Allow handles to be disposed and WebKit objects to be dereferenced even if their map is already destroyed.
+ https://bugs.webkit.org/show_bug.cgi?id=42634
+
+ Currently DOMDataStore could be destroyed even if it has some mappings (it gets destroyed
+ when its isolated context gets GCed). However in this case, handles allocated for
+ such objects would never be disposed as we require presence of mapping from wrapped
+ WebKit object to handle being collected in the map and now map is gone. That leads to
+ zombie objects in both WebKit (wrapped WebKit object doesn't get dereferenced) and V8
+ (both handle and V8 wrapper object could not be destroyed).
+
+ See http://code.google.com/p/chromium/issues/detail?id=47125 for further discussion.
+
+ * bindings/v8/DOMData.h:
+ (WebCore::DOMData::handleWeakObject):
+ * bindings/v8/DOMDataStore.cpp:
+ (WebCore::DOMDataStore::weakNodeCallback):
+ * bindings/v8/V8DOMMap.h:
+ (WebCore::WeakReferenceMap::~WeakReferenceMap):
+
2010-07-20 Hans Wennborg <hans at chromium.org>
Reviewed by Steve Block.
diff --git a/WebCore/bindings/v8/DOMData.h b/WebCore/bindings/v8/DOMData.h
index 4d7331b..c8b4a41 100644
--- a/WebCore/bindings/v8/DOMData.h
+++ b/WebCore/bindings/v8/DOMData.h
@@ -79,14 +79,25 @@ namespace WebCore {
template<typename T>
void DOMData::handleWeakObject(DOMDataStore::DOMWrapperMapType mapType, v8::Persistent<v8::Object> v8Object, T* domObject)
{
+ WrapperTypeInfo* type = V8DOMWrapper::domWrapperType(v8Object);
DOMDataList& list = DOMDataStore::allStores();
+ bool found = false;
for (size_t i = 0; i < list.size(); ++i) {
DOMDataStore* store = list[i];
ASSERT(store->domData()->owningThread() == WTF::currentThread());
DOMWrapperMap<T>* domMap = static_cast<DOMWrapperMap<T>*>(store->getDOMWrapperMap(mapType));
- if (domMap->removeIfPresent(domObject, v8Object))
- store->domData()->derefObject(V8DOMWrapper::domWrapperType(v8Object), domObject);
+ if (domMap->removeIfPresent(domObject, v8Object)) {
+ derefObject(type, domObject);
+ found = true;
+ }
+ }
+
+ // If not found, it means map for the wrapper has been already destroyed, just dispose the
+ // handle and deref the object to fight memory leak.
+ if (!found) {
+ v8Object.Dispose();
+ derefObject(type, domObject);
}
}
diff --git a/WebCore/bindings/v8/DOMDataStore.cpp b/WebCore/bindings/v8/DOMDataStore.cpp
index 5d609d8..0b06a69 100644
--- a/WebCore/bindings/v8/DOMDataStore.cpp
+++ b/WebCore/bindings/v8/DOMDataStore.cpp
@@ -163,10 +163,15 @@ void DOMDataStore::weakNodeCallback(v8::Persistent<v8::Value> v8Object, void* do
DOMDataStore* store = list[i];
if (store->domNodeMap().removeIfPresent(node, v8Object)) {
ASSERT(store->domData()->owningThread() == WTF::currentThread());
- node->deref(); // Nobody overrides Node::deref so it's safe
- break; // There might be at most one wrapper for the node in world's maps
+ node->deref(); // Nobody overrides Node::deref so it's safe
+ return; // There might be at most one wrapper for the node in world's maps
}
}
+
+ // If not found, it means map for the wrapper has been already destroyed, just dispose the
+ // handle and deref the object to fight memory leak.
+ v8Object.Dispose();
+ node->deref(); // Nobody overrides Node::deref so it's safe
}
bool DOMDataStore::IntrusiveDOMWrapperMap::removeIfPresent(Node* obj, v8::Persistent<v8::Data> value)
diff --git a/WebCore/bindings/v8/V8DOMMap.h b/WebCore/bindings/v8/V8DOMMap.h
index a7e03a0..d8d5c04 100644
--- a/WebCore/bindings/v8/V8DOMMap.h
+++ b/WebCore/bindings/v8/V8DOMMap.h
@@ -74,13 +74,7 @@ namespace WebCore {
public:
typedef AbstractWeakReferenceMap<KeyType, ValueType> Parent;
WeakReferenceMap(v8::WeakReferenceCallback callback) : Parent(callback) { }
- virtual ~WeakReferenceMap()
- {
- #ifndef NDEBUG
- if (m_map.size() > 0)
- fprintf(stderr, "Leak %d JS wrappers.\n", m_map.size());
- #endif
- }
+ virtual ~WeakReferenceMap() { }
// Get the JS wrapper object of an object.
virtual v8::Persistent<ValueType> get(KeyType* obj)
@@ -135,7 +129,6 @@ namespace WebCore {
protected:
HashMap<KeyType*, ValueType*> m_map;
- v8::WeakReferenceCallback m_weakReferenceCallback;
};
template <class KeyType> class DOMWrapperMap : public WeakReferenceMap<KeyType, v8::Object> {
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list