Opened 14 years ago

Closed 14 years ago

#5368 closed Bug (fixed)

Memory leak in setData function

Reported by: oarnault Owned by: Alfonso Martínez de Lizarrondo
Priority: Normal Milestone: CKEditor 3.3
Component: General Version: 3.0
Keywords: Review+ Cc: oarnault@…

Description

Hi

I think there is a memory leak in the function CKEDITOR.editor.setData().

OS : WIndow 7 Firefox 3.6

I create an editor and a button. When I click the button I put 10 time 1 million 'a' in the editor using the function setData. At the beginning my brower took 69MO , after 10 click on the button it goes 330MO. (I have waiting for the GC to collect).

It is a problem for me because I'm using the ckeditor in an ajax application and so never reloaded the page moreover my customer does big Word paste into the editor.

I join my simple html witch illustate that.

Thanks.

Attachments (2)

memoryLeak.html​ (1.0 KB) - added by oarnault 14 years ago.
5368.patch​ (1.4 KB) - added by Alfonso Martínez de Lizarrondo 14 years ago.
Proposed patch

Download all attachments as: .zip

Change History (10)

Changed 14 years ago by oarnault

Attachment: memoryLeak.html​ added

comment:1 Changed 14 years ago by oarnault

Cc: oarnault@… added

comment:2 Changed 14 years ago by Alfonso Martínez de Lizarrondo

Resolution: → duplicate
Status: new → closed

If you test the current code you'll see that there's almost no increase after fixing #4555. Also, the test is too rude and it should wait for the new data to have been set before trying to change it again.

This avoids to hang the browser with the test: 

	var counter=0

	function fillTextArea() {
		if ( ++counter > 10 ) 
			return;
		
		document.title=counter;
		window.CKEDITOR.instances['editor1'].setData(value, fillTextArea);
	}

comment:3 Changed 14 years ago by oarnault

I update to nightly build and use the following code 

<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<title>CKEditor memory leak</title>
<meta content="text/html; charset=utf-8" http-equiv="content-type" />
<script type="text/javascript" src="ckeditor/ckeditor.js"></script>

<script type="text/javascript">
	var value = "";
	for ( var i = 1; i < 1000000; i++) {
		value += "a";
		if (i % 100 == 0) {
			value += "<br />";
		}
	};

	
	var counter=0

	function process() {
		counter=0;
		fillTextArea();
	}
	
	function fillTextArea() {
		if ( ++counter > 10 ) 
			return;
		
		document.title=counter;
		window.CKEDITOR.instances['editor1'].setData(value, fillTextArea);
	}

	
</script>

</head>


<body onload="CKEDITOR.replace( 'editor1'); window.CKEDITOR.instances['editor1'].setData(value) ">

<button onclick="process()">Put 10 times 1 million 'a' in textArea</button>
<textarea id="editor1">
</textarea>
</body>
</html>

I loose about 30MO by click on the button.

comment:4 in reply to:  3 Changed 14 years ago by oarnault

Replying to oarnault:

I update to nightly build and use the following code 

<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<title>CKEditor memory leak</title>
<meta content="text/html; charset=utf-8" http-equiv="content-type" />
<script type="text/javascript" src="ckeditor/ckeditor.js"></script>

<script type="text/javascript">
	var value = "";
	for ( var i = 1; i < 1000000; i++) {
		value += "a";
		if (i % 100 == 0) {
			value += "<br />";
		}
	};

	
	var counter=0

	function process() {
		counter=0;
		fillTextArea();
	}
	
	function fillTextArea() {
		if ( ++counter > 10 ) 
			return;
		
		document.title=counter;
		window.CKEDITOR.instances['editor1'].setData(value, fillTextArea);
	}

	
</script>

</head>


<body onload="CKEDITOR.replace( 'editor1'); window.CKEDITOR.instances['editor1'].setData(value) ">

<button onclick="process()">Put 10 times 1 million 'a' in textArea</button>
<textarea id="editor1">
</textarea>
</body>
</html>

I loose about 30MO by click on the button.

I wait the counter reach 10 before click again

comment:5 Changed 14 years ago by Alfonso Martínez de Lizarrondo

Resolution: duplicate
Status: closed → reopened

That's funny. Seems that I tested IE or didn't look clearly at the data. There are still leaks, but fortunately thanks to #5317 it was easier to find them.

Well, find them mostly...
Before the patch I get this usage after each set of runs: 

60.9	93.7	113.0	133

And now it's like this: 

62.5	75.3	82.2	82.9	81.4	83.0	82.4

There's still an increase on first runs, (but much lower), and then it seems to stabilize a little over the 82Mb, and if it was a real leak then it should keep increasing always.

So for the moment I'm not gonna try to find out if there's another extra leak because I don't think so given the current data.

Changed 14 years ago by Alfonso Martínez de Lizarrondo

Attachment: 5368.patch​ added

Proposed patch

comment:6 Changed 14 years ago by Alfonso Martínez de Lizarrondo

Keywords: Review? added
Owner: set to Alfonso Martínez de Lizarrondo
Status: reopened → new

comment:7 Changed 14 years ago by Garry Yao

Keywords: Review+ added; Review? removed
Milestone: → CKEditor 3.3
Version: → 3.0

Please rename 'clearCustomData' to 'onDispose' before commit, we would introduce such convention for leak protection in other places.

comment:8 Changed 14 years ago by Alfonso Martínez de Lizarrondo

Resolution: → fixed
Status: new → closed

Fixed with [5289]. The #4555 change log entry is enough for this.

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy