adamlytics / svgweb

Automatically exported from code.google.com/p/svgweb
Other
0 stars 1 forks source link

Eliminate memory leaks reported by IEJSLeaksDetector.exe #604

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. Download and install IEJSLeaksDetector.exe from Microsoft.
2. Run up a multiple-page web site containing SVGWeb.
3. Note the ondocumentready handler "leaks" (isn't properly removed in all 
cases).

Using Feb 2 version of svgweb, and prior versions with Flash 10x.

The repair appears to be:

In the documentReady = function()... logic moving the detachEvent() call 
outside of the if (window.onload) test.

Also, in the _fireUnload function, it seems appropriate to add:

    window.svgweb = null;

ss

Original issue reported on code.google.com by Scott.Sh...@gmail.com on 15 Feb 2011 at 6:58

GoogleCodeExporter commented 9 years ago
This can be easily replicated by using even the hosted javascript samples.

1. Download and install the IEJSLeaksDetector.exe from Microsoft 
(http://blogs.msdn.com/b/gpde/archive/2009/08/03/javascript-memory-leak-detector
-v2.aspx)

2. Using the Leaks Detector, browse to the tetris sample hosted on coding 
paradise 
(http://codinginparadise.org/projects/svgweb/samples/javascript-samples/blocks_g
ame.html?svg.render.forceflash=true)

3. Hit "stop" on the leaks detector, and numerous leaks are noted in the 
interface.

Original comment by joech...@gmail.com on 15 Feb 2011 at 11:43

GoogleCodeExporter commented 9 years ago
Another potential example is using the "Blocks" game noted in Issue 99.

Keeping a task manager open, and then refreshing the game over and over, you 
can watch the memory level increase to the point where IE will crash.

The refresh never reclaims memory.  It seems to increase about 10M on the 
iexplore.exe process each refresh.

Willing to help, however, so if you can point us to how Issue 99 was resolved, 
we can take a stab.

Thanks!

Original comment by joech...@gmail.com on 16 Feb 2011 at 2:40

GoogleCodeExporter commented 9 years ago
IT seems like removing the DOCTYPE from the top of the page of all samples, you 
can prevent the memory leak from happening.

Original comment by joech...@gmail.com on 17 Feb 2011 at 12:54

GoogleCodeExporter commented 9 years ago
This could be related to the Microsoft Behaviors. Once a behavior is 
instantiated, even if it is removed from the page, its memory is never 
reclaimed by IE:

http://support.microsoft.com/kb/315014

Microsoft will probably never fix this bug.

One workaround would be for us to recycle detached behaviors inside of SVG Web.

Original comment by bradneub...@gmail.com on 21 Feb 2011 at 11:03

GoogleCodeExporter commented 9 years ago
Tracing this down further, it seems like DOCTYPE was placing the IE8 browser 
into IE8 Standards mode, which accelerates the memory leak on refresh to 10M 
per refresh.

Using IE8 in either Quirks Mode or IE7 Standards mode did not result in 
significant leakage during the 'refresh test'.

If it's IE8 and Standards mode, do you think it's still Behaviors doing the 
dirty work?  If you have existing knowledge on it, we may take a stab at 
writing a "Behavior Pool" so that they are reused.

Original comment by joech...@gmail.com on 23 Feb 2011 at 2:45

GoogleCodeExporter commented 9 years ago
We specify the HTML 5 DOCTYPE on our examples which should trigger standards 
mode.

If IE 8 is in standards mode, then Object.defineProperty should be available 
and it should use that instead of Behaviours. So, I have no idea what this 
memory leak is but it is probably not the behaviours.

Original comment by grick23@gmail.com on 24 Feb 2011 at 4:41

GoogleCodeExporter commented 9 years ago
Just wanted to note that this is a fairly serious bug in my use case which can 
create a number of svg roots on a page and could have many pages with svg. The 
site uses standards mode for other layout reasons. 

I am surprised this is a Low priority issue. A 10MB leak per svg root can grow 
substantial quickly.

BTW, In my cursory experiment I believe I saw leaks in both modes on IE8. Are 
you sure there's no leak in quirks mode? If there is no leak in IE8 quirks mode 
I may be creating one of my own, though I do call svgweb.removeChild and the 
leak seems to persist even after page reload.

Original comment by marki...@gmail.com on 1 Mar 2011 at 5:14

GoogleCodeExporter commented 9 years ago
Let me retract part of my last comment. The leak does seem to be restricted to 
standards mode on IE8.

Original comment by marki...@gmail.com on 1 Mar 2011 at 8:01

GoogleCodeExporter commented 9 years ago
Changing to high priority.

Original comment by grick23@gmail.com on 1 Mar 2011 at 8:34

GoogleCodeExporter commented 9 years ago
I guess this patch may fix this problem (at least partially):

--- src/svg.js  (revision 1336)
+++ src/svg.js  (working copy)
@@ -751,7 +751,7 @@
   /** Every element (including text nodes) has a unique GUID. This lookup
       table allows us to go from a GUID taken from an XML node to a fake 
       node (_Element or _Node) that might have been instantiated already. */
-  _guidLookup: [],
+  _guidLookup: {},

   /** Onload page load listeners. */
   _loadListeners: [],
@@ -1060,6 +1060,9 @@
         // SVG OBJECT
         for (var guid in svgweb._guidLookup) {
           var child = svgweb._guidLookup[guid];
+            if (child._fake) {
+              child._htcNode = null;
+            }
           if (child._fake && child.ownerDocument === nodeHandler.document) {
             delete svgweb._guidLookup[guid];
           }

Original comment by Hsia.X....@gmail.com on 1 Apr 2011 at 6:00

GoogleCodeExporter commented 9 years ago
The IE8-refreshing-issue is caused by a memory leak in SVGWeb.removeChild 
function. While there is another memory leak in SVGWeb._fireUnload function 
which could lead to similar problem when invoking replaceChild or removeChild 
method of a node.

These memory leaks are fixed by the following patch.

===================================================================
--- src/svg.js  (revision 1336)
+++ src/svg.js  (working copy)
@@ -751,7 +751,7 @@
   /** Every element (including text nodes) has a unique GUID. This lookup
       table allows us to go from a GUID taken from an XML node to a fake 
       node (_Element or _Node) that might have been instantiated already. */
-  _guidLookup: [],
+  _guidLookup: {},

   /** Onload page load listeners. */
   _loadListeners: [],
@@ -1060,6 +1060,9 @@
         // SVG OBJECT
         for (var guid in svgweb._guidLookup) {
           var child = svgweb._guidLookup[guid];
+            if (child._fake) {
+              child._htcNode = null;
+            }
           if (child._fake && child.ownerDocument === nodeHandler.document) {
             delete svgweb._guidLookup[guid];
           }
@@ -2163,6 +2166,7 @@
       }
       node._fakeNode = null;
       node._handler = null;
+      svgweb._removedNodes[i] = null;
     }
     svgweb._removedNodes = null;

Original comment by Hsia.X....@gmail.com on 7 Apr 2011 at 4:32