Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Memory leak in Array.splice() #3035

Closed
ChristianStroyer opened this issue May 24, 2017 · 6 comments
Closed

Memory leak in Array.splice() #3035

ChristianStroyer opened this issue May 24, 2017 · 6 comments
Assignees
Labels
Milestone

Comments

@ChristianStroyer
Copy link

In some cases an object removed from an array with Array.splice is still allocated in memory, even though it is not seen in a heap dump. This results in IE(11.0.42) and Edge(38.14393) running out of memory in Windows in our web app. Chrome(58) does not have this issue.

Array.splice is a common way to remove entries in arrays. If it leaks I believe it is a huge issue for Edge.

I've created a small example. It creates an array with a large objects, and then removes the object with splice(0,1). Running this for a few seconds takes both IE and Edge to a memory consumption of more than 1GB, which is never freed. An inspection shows only empty arrays.

Example at https://jsfiddle.net/aefxf0v9/

// create 80kb Object
function createLargeObject(){
	var temparray = [];
	for(var i=0;i<10000;i++){
		temparray[i]=i;
	}
  return { obj:temparray };
}

function leak() { 
	globalarray = [];
	for(var i=0;i<20000;i++) {
		globalarray[i]=[];
		var innerarray = globalarray[i];
		var largeObj = createLargeObject();
		innerarray[0] = largeObj ;
		//innerarray[0] = null;   // Adding this will fix the problem
		innerarray.splice( 0, 1 );  
	}
}

Setting the array entry to null before removing it with splice fixes the problem. I have made a shim for our project, but would very much like to see a solution.

@ChristianStroyer
Copy link
Author

Exploring a bit further it seems the problem gravitates around removing the final entry in an array. The following examples supports it:

// Leak
innerarray[0] = largeObj ;
innerarray.splice( 0, 1 );  
// No issue
innerarray[0] = 0 ;
innerarray[1] = largeObj ;
innerarray.splice( 1, 1 );  
// No issue
innerarray[0] = largeObj ;
innerarray[1] = 0 ;
innerarray.splice( 0, 1 ); 
// Leak
innerarray[0] = 0 ;
innerarray[1] = largeObj ;
innerarray.splice( 0, 2 );  

@curtisman curtisman added this to the 1.6 milestone May 24, 2017
@michielcrivits
Copy link

We are experiencing a memory leak in our web application as of IE v11.0.42. We have a strong feeling it's related to this bug. Any idea yet when this would be solved in IE 11?

Are there certain ways to avoid/lower the impact of the issue? IE settings, alternatives, ...

@suwc
Copy link

suwc commented Jun 12, 2017

Thanks for the feedback. Our plan is to have the fix in our next release (Windows 10 Update). I will let you know if a work-around is available.

@michielcrivits
Copy link

Thanks for the fast response! Are we talking about weeks/months/half a year? What about IE 11 on Windows 7/8?

@suwc
Copy link

suwc commented Jun 12, 2017

Unfortunately, only security fixes will be made (monthly) for IE11 on Windows 7/8, which is not applicable here. Looks like only next Windows 10 Update will have the fix.

As @ChristianStroyer mentioned, one work-around is to set the array entry to null before using splice() to remove those entries; another work-around is to start with index 1.

@ChristianStroyer
Copy link
Author

Fixing IE leak and Edge leak:

Here is the workaround we use in our project. It's a so called shim which overrides the existing array.splice and substitutes it with a version which nulls out elements before calling the original splice.
Adding this to your project has the added benefit of fixing this issue in third party libraries having the same issue.

Try it on https://jsfiddle.net/aefxf0v9/6/

function setupShim() {

		console.log("Shimming Array.splice()");
		
		var oldSplice = [].__proto__.splice;
		
		var newSplice = function(start, deleteCount ) {		
			if( start < 0
				|| start > this.length - 1 
				|| deleteCount === undefined 
				|| deleteCount < 0 
				|| start + deleteCount > this.length 
				|| arguments.length > 2 ) {
					// Nasty corner cases. Using old splice.
					return oldSplice.apply(this, arguments); 
			}	
			var retval = new Array(deleteCount);
			for(var i = 0 ; i < deleteCount && i < this.length; i++) {
				retval = this[start+i];
				this[start+i] = null;
			}
			oldSplice.apply(this, arguments); 
			return retval;
		};
		
		[].__proto__.splice = newSplice;

}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants