Full Text Bug Listing

Flament David

Reported 2009-11-09 05:15:54 PST

Created attachment 42748 [details] bug_memoryLeak.html Hi, I use a test page that will continuously consume memory until there's no free memory. I looked into the test page's source, the javascript code is doing such a process: 1) create an image with "new Image()"; 2) set the image.src to one of the 12 test pictures; 3) append the image to document container; 4) continue the loop from 1) This piece of test code consume more and more memory until there's no free memory. So, I think, Garbage collector management failed. Have you an idea ? Bug already exist ? Exemple of page using in attachement. Thank's for your help ... Thank

Alexey Proskuryakov

Comment 1 2009-11-09 11:59:30 PST

I do see unbounded memory growth, and the memory isn't released even after closing the page. Tested with a local debug build of r50532 on Mac OS X 10.5.8 (so, it's cross-platform).

Flament David

Comment 2 2009-11-10 00:31:36 PST

Comment on attachment 42748 [details] bug_memoryLeak.html <html> <head> <script language="javascript"> var containerNode, compteur=0, lastImg, timer, dist=45, number=0; function init(){ var img = new Image(); img.src = "./resources/"+compteur+".jpg"; lastImg = img; containerNode = document.getElementById("imgContainer"); containerNode.appendChild(img); } function start_loop(){ timer = window.setInterval(function(){ rLaunch(); },1); } function stop_loop(){ clearInterval(timer); } function rLaunch(){ document.getElementById("imgContainer").innerHTML = ""; lastImg=null; dist = dist+1; number = number+1; var img = new Image(); compteur = (compteur==12) ? 0 : compteur+1; img.src = "./resources/"+compteur+".jpg?dist="+dist; lastImg = img; containerNode.appendChild(img); document.getElementById("textContainer").innerHTML = "Images loaded : "+ number; } function clean(){ document.getElementById("imgContainer").innerHTML = ""; number = 0; clearInterval(timer); // add by DFL while(containerNode.hasChildNodes()) { containerNode.removeChild(containerNode.lastChild) ; } // Remove element nodes and prevent memory leaks jQuery(this).children().remove(); // Remove any remaining nodes while (this.firstChild) this.removeChild(this.firstChild); containerNode.clear(); containerNode.empty(); } </script> <style type="text/css"> img{ left:45px; top:45px; position:absolute; } .opacity img{ opacity:0.5; } div.controls{ position:absolute; bottom:20px; left:45px; } div.textContainer{ position:absolute; top: 150px; left: 400px; } </style> </head> <body onload="init()"> <div id="controls" class="controls"> <a href="#" onclick="javascript:start_loop()">Start_loop</a> <a href="#" onclick="javascript:stop_loop()">Stop_loop</a> </div> <div id="imgContainer"></div> <div id="textContainer" class="textContainer"></div> </body> </html>

Mihnea Ovidenie

Comment 3 2010-07-08 06:33:00 PDT

Hi, I have seen the same behavior while trying to add/remove several images into DOM tree. After some time spent in debugging, i have noticed that DocLoader is keeping references to CachedResources in its member m_documentResources. The type of m_documentResources is typedef HashMap<String, CachedResourceHandle<CachedResource> > and it was introduced with changelists 39725 and 39304(bug 17998). When an image is removed from DOM, DocLoader is still keeping a reference to it even when the resource is purged from cache. It is true that when DocLoader is destroyed, the CachedResources from m_documentResources are removed since their reference count is dropping to 0. Using a similar test as the one already posted in the bug (adding/removing a lot of images) both WebKit nightly and Safari 5 have their memory grow to 1GB on Mac/Win after 1 day of running. Chrome does not have this problem, Firefox 3.6.6 does not have this problem, IE 8 also does not have this problem. I was wondering if the DocLoader still keeping a reference to all CachedResources could be the cause for this memory growth. I was experimenting a small patch with DocLoader keeping a weak reference to its CachedResources, so that these resources get the chance to be deleted when they are removed from cache. I also saw that there is another bug logged to an issue that may be related to this one: https://bugs.webkit.org/show_bug.cgi?id=23372 Repeatedly overwriting img.src can cause memory exhaustion. Could someone comment on this? Regards, Mihnea Ovidenie

Mihnea Ovidenie

Comment 4 2010-07-08 08:16:50 PDT

Created attachment 60885 [details] Simple patch, possible solution to improve memory consumption Here is a simple patch that implements the idea of DocLoader keeping a weak reference to its CachedResources. When this resources are removed from cache, they are also deleted since DocLoader will not keep a reference to them. Taking into account the complexity of such issue, it would be great if somebody with expertise could take a look and share an opinion about this patch. Regards, Mihnea

Alexey Proskuryakov

Comment 5 2010-07-08 10:20:18 PDT

Comment on attachment 60885 [details] Simple patch, possible solution to improve memory consumption I didn't look at the patch, but I assume that it implements what the above comment describes. The reason for m_documentResources existence is so that we could provide a list of resources to clients via -[WebDataSource subresources], store them in Web archives, and show them in Web Inspector. Thus, removing items just because the resource data is no longer cached is inappropriate.

Mihnea Ovidenie

Comment 6 2010-07-08 23:22:14 PDT

(In reply to comment #5) > (From update of attachment 60885 [details]) > I didn't look at the patch, but I assume that it implements what the above comment describes. > > The reason for m_documentResources existence is so that we could provide a list of resources to clients via -[WebDataSource subresources], store them in Web archives, and show them in Web Inspector. Thus, removing items just because the resource data is no longer cached is inappropriate. Thanks a lot for your comment.

Alexey Proskuryakov

Comment 7 2010-11-11 15:48:07 PST

See also: bug 39628.

Kevin Gadd

Comment 8 2010-12-16 16:57:36 PST

(In reply to comment #5) > (From update of attachment 60885 [details]) > I didn't look at the patch, but I assume that it implements what the above comment describes. > > The reason for m_documentResources existence is so that we could provide a list of resources to clients via -[WebDataSource subresources], store them in Web archives, and show them in Web Inspector. Thus, removing items just because the resource data is no longer cached is inappropriate. I have recently discovered that this bug makes an application I'm building unusable on any modern WebKit-based browser, as it exhausts available memory within minutes. I've yet to find a workaround, so I decided to come see if perhaps I could at least submit a patch that fixes the bug and hope that it makes its way to devices like the iPhone in a reasonable amount of time. Am I interpreting your statement correctly that you believe that this memory leak is intended behavior and should be preserved simply to keep Web Inspector's current behavior? I understand the desire to have web inspector offer a list of all resources ever loaded by a page, but doing it in this way breaks a large set of web development use cases. This seems an extreme cost to pay merely to get a feature of questionable value within Web Inspector. At the very least, this unbounded memory consumption should only be triggered when resource tracking has been explicitly requested by the user. Is there a way that this bug can be fixed and still meet your functionality requirements for Web Inspector? If you can describe the requirements for an acceptable fix, I can attempt to create that fix.

Alexey Proskuryakov

Comment 9 2010-12-16 17:37:00 PST

Note that Web Inspector was not the only client that I mentioned as needing access to document resources. Anyway, I don't think that anyone will argue that secondary functionality (like Activity window or web archives) should prevent web content from working. It may be just fine to remove resources from m_documentResources when all elements using them are known to have been removed from the document, and won't come back from a detached tree. It's not very easy though, as external resources can also be referenced by style rules, so counting only elements isn't sufficient. There is some surprising code in CachedResourceLoader today. The attached patch implies that CachedResourceLoader holds a strong reference to document resources - but why does this class have a removeCachedResource() method called from CachedResource destructor then? It's not obvious that all bugs with memory growth when adding and removing images are the same internally. There may be loader problems, but there may also be JavaScript garbage collection problems. Unless you're 100% sure that you are seeing an identical issue, it may be best to file a separate bug - even if it's found to be a duplicate later, that's much better than discussing two issues in one bug.

Chen Zhixiang

Comment 10 2013-08-14 22:43:20 PDT

The patch doesn't work for our WinCE port which is based on a 2012/8 nearby WebKit. Test code doesn't add/remove <img> nodes, but just keep changing <img>'s src attribute to load different images. The debug output says: "LEAK: 85 CachedResource", which is the all image resources. Also, the newest Chrome 28 and Firefox 23 also both have this problem.

Mihnea Ovidenie

Comment 11 2013-08-14 23:39:59 PDT

(In reply to comment #10) > The patch doesn't work for our WinCE port which is based on a 2012/8 nearby WebKit. > > Test code doesn't add/remove <img> nodes, but just keep changing <img>'s src attribute to load different images. > > The debug output says: "LEAK: 85 CachedResource", which is the all image resources. > > Also, the newest Chrome 28 and Firefox 23 also both have this problem. Well, you can always propose a new patch/approach if the problem still persist.

Arunprasad Rajkumar

Comment 12 2014-01-19 21:45:53 PST

(In reply to comment #11) > (In reply to comment #10) > > The patch doesn't work for our WinCE port which is based on a 2012/8 nearby WebKit. > > > > Test code doesn't add/remove <img> nodes, but just keep changing <img>'s src attribute to load different images. > > > > The debug output says: "LEAK: 85 CachedResource", which is the all image resources. > > > > Also, the newest Chrome 28 and Firefox 23 also both have this problem. > > Well, you can always propose a new patch/approach if the problem still persist. I think http://trac.webkit.org/changeset/92143 fixed some part of this issue.

Makivelli

Comment 13 2015-02-16 23:02:39 PST

Comment on attachment 42748 [details] bug_memoryLeak.html ><html> > <head> > > <script language="javascript"> > > var containerNode, compteur=0, lastImg, timer, dist=45, number=0; > var activateGfx = false; > var activateOpacity = false; > > function init(){ > var img = new Image(); > img.src = "./resources/"+compteur+".jpg"; > lastImg = img; > containerNode = document.getElementById("imgContainer"); > containerNode.appendChild(img); > } > > function start(){ > timer = window.setInterval(function(){ > rLaunch(); > },1); > } > > function stop(){ > clearInterval(timer); > } > > function rLaunch(){ > document.getElementById("imgContainer").innerHTML = ""; > //lastImg.style.right=dist+"px"; > lastImg=null; > dist = dist+1; > number = number+1; > //lastImg.style.left="auto"; > var img = new Image(); > compteur = (compteur==12) ? 0 : compteur+1; > var str = activateGfx ? "|com.motorola.gfxmem=true":""; > img.src = "./resources/"+compteur+".jpg?dist="+dist+str; > lastImg = img; > > containerNode.appendChild(img); > var desc = activateOpacity ? " opacity activated" : ""; > document.getElementById("textContainer").innerHTML = "NbImages chargees : "+ number + desc; > } > > function clean(){ > document.getElementById("imgContainer").innerHTML = ""; > number = 0; > clearInterval(timer); > > // add by DFL > while(containerNode.hasChildNodes()) { > containerNode.removeChild(containerNode.lastChild) ; > } > > // Remove element nodes and prevent memory leaks > jQuery(this).children().remove(); > // Remove any remaining nodes > while (this.firstChild) > this.removeChild(this.firstChild); > > containerNode.clear(); > containerNode.empty(); > > } > > function activOpacity(){ > activateOpacity = !activateOpacity; > if(activateOpacity) { > document.getElementById("imgContainer").className = "opacity"; > document.getElementById("isOpacityActivated").innerHTML = "(on)"; > } else { > document.getElementById("imgContainer").className = ""; > document.getElementById("isOpacityActivated").innerHTML = "(off)"; > } > > clean(); > } > </script> > > <style type="text/css"> > > img{ > left:45px; > top:45px; > position:absolute; > /* opacity:0.5;*/ > } > > .opacity img{ > opacity:0.5; > } > > div.controls{ > position:absolute; > bottom:20px; > left:45px; > > } > > div.textContainer{ > position:absolute; > top: 150px; > left: 400px; > } > > > </style> > > > </head> > <body onload="init()"> > <div id="controls" class="controls"> > <a href="#" onclick="javascript:start()">Start</a> > <a href="#" onclick="javascript:stop()">Stop</a> > <a href="#" onclick="javascript:activOpacity()">Opacity <span id="isOpacityActivated">(Off)</span></a> > </div> > <div id="imgContainer"></div> > <div id="textContainer" class="textContainer"></div> > </body> ></html>

Pierre Fritsch

Comment 14 2018-07-19 00:47:17 PDT

For others battling with the memory increase in Safari when changing an image src, there's a work around until this bug is fixed: Store the image binary data in a `Blob` object, `createObjectURL` to represent this blob, and assign this URL as the `Image.src`. Finally, `revokeObjectURL` before switching to the next image. See also https://stackoverflow.com/a/38624675/4769344 Note that this method assumes the usage of Data URIs. If you need to read existing image files, you would have to load the images on some server and to pass them to the browser as data URIs in order to avoid the creation of `Image` objects in WebKit.

Avner

Comment 15 2022-02-15 22:51:24 PST

It seems like this is still a problem. In my case I read a blob from a local file, by extracting a zip entry from a zip file, without the use of an "Image" object. A blob is then created from the array, and managed with createObjectURL, and revokeObjectURL. But memory leak still happens on browsers in iOS, so I assume this is a bug in Webkit. The problem is described in https://stackoverflow.com/questions/71122643/webapp-fails-in-ios-when-using-revokeobjecturl and https://developer.apple.com/forums/thread/700639 and an online example that demonstrates the problem is in https://jsfiddle.net/avnerm/vfgkdenp/18/

Avner

Comment 16 2022-02-16 19:26:29 PST

I simplified the problem (which happens on iOS): - replaced reading-from-zip with reading-from-external-url - mimicked the reading of blob by artificially using createObjectURL, and revokeObjectURL. The problem is described and demonstrated in: https://stackoverflow.com/questions/71151958/memory-leak-in-ios-when-loading-images-from-external-url-and-using-createobjectu

Attachments
bug_memoryLeak.html (2.76 KB, text/html) 2009-11-09 05:15 PST, Flament David	no flags	Details
Simple patch, possible solution to improve memory consumption (13.21 KB, patch) 2010-07-08 08:16 PDT, Mihnea Ovidenie	ap: review-	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Bug 31253