Merge pull request #566 from donejs/mem-leak

Clean up memory correctly

package.json

@@ -47,7 +47,7 @@
     "he": "^1.1.1",
     "jquery": "2.x - 3.x",
     "jshint": "^2.8.0",
-    "leakage": "0.3.0-beta",
+    "leakage": "0.4.0",
     "nock": "^9.0.11",
     "spawn-mochas": "^1.1.0",
     "steal-stache": "^4.0.0",
@@ -56,8 +56,8 @@
     "uglify-js": "^3.3.19"
   },
   "dependencies": {
-    "can-dom-data-state": "^1.0.1",
-    "can-dom-mutate": "^1.0.2",
+    "can-dom-data-state": "^1.0.2",
+    "can-dom-mutate": "^1.2.1",
     "can-namespace": "^1.0.0",
     "can-reflect": "^1.11.0",
     "can-util": "^3.10.10",

test/async_test.js

@@ -14,9 +14,10 @@ describe("async rendering", function(){
 		});
 
 		helpers.createServer(8070, function(req, res){
+			var data;
 			switch(req.url) {
 				case "/bar":
-					var data = [ { "a": "a" }, { "b": "b" } ];
+					data = [ { "a": "a" }, { "b": "b" } ];
 					break;
 				default:
 					throw new Error("No route for " + req.url);

test/auth-cookie-failed-domain_test.js

@@ -24,10 +24,11 @@ describe("auth cookies - failed domain", function() {
 		});
 
 		helpers.createServer(8070, function(req, res){
+			var data;
 			authHeader = req.headers.authorization;
 			switch(req.url) {
 				case "/api/list":
-					var data = [1,2,3,4,5];
+					data = [1,2,3,4,5];
 					break;
 				default:
 					throw new Error("No route for " + req.url);

test/auth-cookie_test.js

@@ -25,10 +25,11 @@ describe("auth cookies", function() {
 		});
 
 		helpers.createServer(8070, function(req, res){
+			var data;
 			authHeader = req.headers.authorization;
 			switch(req.url) {
 				case "/api/list":
-					var data = [1,2,3,4,5];
+					data = [1,2,3,4,5];
 					break;
 				default:
 					throw new Error("No route for " + req.url);

test/incremental_prog_test.js

@@ -9,9 +9,10 @@ describe("Incremental rendering", function(){
 
 	before(function(done){
 		helpers.createServer(8070, function(req, res){
+			var data;
 			switch(req.url) {
 				case "/bar":
-					var data = [ { "a": "a" }, { "b": "b" } ];
+					data = [ { "a": "a" }, { "b": "b" } ];
 					break;
 				default:
 					throw new Error("No route for " + req.url);

test/incremental_test.js

@@ -12,9 +12,10 @@ describe("Incremental rendering", function(){
 
 	before(function(done){
 		helpers.createServer(8070, function(req, res){
+			var data;
 			switch(req.url) {
 				case "/bar":
-					var data = [ { "a": "a" }, { "b": "b" } ];
+					data = [ { "a": "a" }, { "b": "b" } ];
 					break;
 				default:
 					throw new Error("No route for " + req.url);

test/leak_test.js

@@ -9,9 +9,24 @@ describe("Memory leaks", function(){
 	this.timeout(30000);
 
 	before(function(done){
-		this.oldXHR = global.XMLHttpRequest;
-		global.XMLHttpRequest = helpers.mockXHR(
-			'[ { "a": "a" }, { "b": "b" } ]');
+		helpers.createServer(8070, function(req, res){
+			var data;
+			switch(req.url) {
+				case "/bar":
+					data = [ { "a": "a" }, { "b": "b" } ];
+					break;
+				default:
+					throw new Error("No route for " + req.url);
+			}
+			res.setHeader("Content-Type", "application/json");
+			res.end(JSON.stringify(data));
+		})
+		.then(server => {
+			this.server = server;
+
+			// Render once so that everything is loaded
+			this.render("/").then(_ => done());
+		});
 
 		var render = ssr({
 			config: "file:" + path.join(__dirname, "tests", "package.json!npm"),
@@ -21,25 +36,33 @@ describe("Memory leaks", function(){
 		this.render = function(pth){
 			return new Promise(function(resolve, reject){
 				var stream = through(function(buffer){
-					setTimeout(resolve, 300);
+					setTimeout(() => resolve(buffer), 300);
 				});
 				stream.on("error", reject);
 				render(pth).pipe(stream);
 			});
 		};
-
-		// Render once so that everything is loaded
-		this.render("/").then(_ => done());
 	});
 
 	after(function(){
-		global.XMLHttpRequest = this.oldXHR;
+		this.server.close();
 	});
 
-	it("do not happen", function(done){
-
+	it("No leaks occur after 10 iterations", function(done){
+		var debug = typeof process.env.DONE_SSR_DEBUG !== "undefined";
+		var cnt = 0;
 		iterate(10, () => {
-			return this.render("/");
+			cnt++;
+			var thisIteration = cnt;
+			if(debug) {
+				console.error("Before render", thisIteration);
+			}
+
+			return this.render("/").then(() => {
+				if(debug) {
+					console.error("After render", thisIteration);
+				}
+			});
 		})
 		.then(() => {
 			done();

test/tests/async/orders/orders.js

@@ -14,7 +14,8 @@ var ViewModel = DefineMap.extend({
 					resolve(data);
 				};
 				xhr.onerror = function(err){
-					console.error(err);
+					var error = err || new Error(this.responseText);
+					console.error(error);
 				};
 				xhr.send();
 			});

test/xhr_test.js

@@ -16,9 +16,10 @@ describe("xhr async rendering", function() {
 		});
 
 		helpers.createServer(8070, function(req, res){
+			var data;
 			switch(req.url) {
 				case "/api/list":
-					var data = [1,2,3,4,5];
+					data = [1,2,3,4,5];
 					break;
 				default:
 					throw new Error("No route for " + req.url);

zones/canjs/cleanup.js

@@ -12,15 +12,15 @@ module.exports = function(data){
 			var domMutate = getDomMutate();
 
 			var docEl = data.document.documentElement;
-			var head = data.document.head;
-			var body = data.document.body;
+
+			// Run the removal within the zone so that the globals point to our globals.
+			var removeDocumentElement = this.wrap(function() {
+				domMutate.removeChild.call(data.document, docEl);
+			});
 
 			// Do this some time later to prevent extra mutations
 			// In the mutation stream
-			later(function(){
-				domMutate.removeChild.call(docEl, head);
-				domMutate.removeChild.call(docEl, body);
-			});
+			later(removeDocumentElement);
 		}
 	};
 };

zones/canjs/globals.js

@@ -7,21 +7,24 @@ module.exports = function(data){
 	}
 
 	var getLocation = getEither.bind(null, "LOCATION", "can-globals/location/location");
-	var getDocument = getEither.bind(null, "DOCUMENT", "can-globals/document/document")
+	var getDocument = getEither.bind(null, "DOCUMENT", "can-globals/document/document");
 
 	var oldLocation, oldDocument;
 
-	return {
-		beforeTask: function(){
-			var LOCATION = getLocation();
-			var DOCUMENT = getDocument();
+	function setCanGlobals() {
+		var LOCATION = getLocation();
+		var DOCUMENT = getDocument();
+
+		oldLocation = LOCATION();
+		LOCATION(data.window.location);
 
-			oldLocation = LOCATION();
-			LOCATION(window.location);
+		oldDocument = DOCUMENT();
+		DOCUMENT(data.window.document);
+	}
 
-			oldDocument = DOCUMENT();
-			DOCUMENT(window.document);
-		},
+	return {
+		afterStealDone: setCanGlobals,
+		beforeTask: setCanGlobals,
 		afterTask: function(){
 			getLocation()(oldLocation);
 			getDocument()(oldDocument);