Changeset 58316 in webkit

Timestamp:

Apr 27, 2010 10:58:56 AM (14 years ago)

Author:

vitalyr@chromium.org

Message:

2010-04-27 Vitaly Repeshko <​vitalyr@chromium.org>

Reviewed by Nate Chapin.

[V8] Fix XHR memory leak in GC controller.
​https://bugs.webkit.org/show_bug.cgi?id=38202

​http://trac.webkit.org/changeset/55798 simplified processing of
active DOM objects but introduced a subtle bug in garbage
collection of XHRs. ActiveDOMObject is not a direct base class of
XHR and so upcasting changes pointer identity and breaks DOM
maps. This leads to forever live persistent handles to XHR that
had pending activity while V8 GC happened.

• bindings/v8/V8GCController.cpp: (WebCore::GCEpilogueVisitor::visitDOMWrapper):

Location:

trunk/WebCore

Files:

• ChangeLog (modified) (1 diff)
• bindings/v8/V8GCController.cpp (modified) (1 diff)

trunk/WebCore/ChangeLog

@@ +1 @@
+2010-04-27  Vitaly Repeshko  <vitalyr@chromium.org>
+
+        Reviewed by Nate Chapin.
+
+        [V8] Fix XHR memory leak in GC controller.
+        https://bugs.webkit.org/show_bug.cgi?id=38202
+
+        http://trac.webkit.org/changeset/55798 simplified processing of
+        active DOM objects but introduced a subtle bug in garbage
+        collection of XHRs. ActiveDOMObject is not a direct base class of
+        XHR and so upcasting changes pointer identity and breaks DOM
+        maps. This leads to forever live persistent handles to XHR that
+        had pending activity while V8 GC happened.
+
+        * bindings/v8/V8GCController.cpp:
+        (WebCore::GCEpilogueVisitor::visitDOMWrapper):
+
 2010-04-27  Adam Barth  <abarth@webkit.org>
 

trunk/WebCore/bindings/v8/V8GCController.cpp

@@ -379 +379 @@
             if (activeDOMObject && activeDOMObject->hasPendingActivity()) {
                 ASSERT(!wrapper.IsWeak());
-                wrapper.MakeWeak(activeDOMObject, &DOMDataStore::weakActiveDOMObjectCallback);
+                // NOTE: To re-enable weak status of the active object we use
+                // |object| from the map and not |activeDOMObject|. The latter
+                // may be a different pointer (in case ActiveDOMObject is not
+                // the main base class of the object's class) and pointer
+                // identity is required by DOM map functions.
+                wrapper.MakeWeak(object, &DOMDataStore::weakActiveDOMObjectCallback);
             }
         }