Skip to content

Commit

Permalink
Merge branch 'hub-memleak'
Browse files Browse the repository at this point in the history
  • Loading branch information
reid committed Dec 20, 2013
2 parents 929a0d2 + bd8cc3b commit f35ab67
Show file tree
Hide file tree
Showing 7 changed files with 103 additions and 39 deletions.
1 change: 1 addition & 0 deletions .travis.yml
@@ -1,6 +1,7 @@
language: node_js
node_js:
- 0.8
- 0.10

before_script:
- "export DISPLAY=:99.0"
Expand Down
55 changes: 39 additions & 16 deletions lib/hub/agent.js
Expand Up @@ -9,9 +9,6 @@ var periodicRegistry = require("./periodic-registry").getRegistry();
var parseUA = require("./ua");
var makeURLFromComponents = require("./url-builder");

// TODO: Periodic GC of missing Agents.
// var periodic = require("./periodic");

/**
* An Agent represents a web browser.
*
Expand Down Expand Up @@ -49,7 +46,7 @@ function Agent(allAgents, registration) {
EventEmitter2.call(this);

// The this.socketEmitter EventYoshi should
// contain at most 1 Socket.io socket.
// contain at most 1 SockJS socket.
//
// We use an EventYoshi so that event
// listeners for sockets only need to be
Expand Down Expand Up @@ -153,6 +150,10 @@ Agent.prototype.setupEvents = function () {
});

self.socketEmitter.on("close", function () {
if (self.destroyed) {
return;
}

self.socketEmitter.remove(this.child);
});

Expand Down Expand Up @@ -185,10 +186,14 @@ Agent.prototype.getName = function () {
};

Agent.prototype.healthCheck = function agentHealthCheck() {
if (this.destroyed) {
return;
}

if (this.currentTestShouldTimeout()) {
this.giveUpOnCurrentTest();
} else if (this.shouldUnload()) {
this.unload();
} else if (this.shouldDestroy()) {
this.destroy();
} else if (!this.seenSince(new Date(this.lastHealthCheck.getTime() - 60000))) {
this.sendPing();
}
Expand Down Expand Up @@ -300,6 +305,10 @@ Agent.prototype.removeTarget = function () {
* @return {String} Next test URL, or capture page URL.
*/
Agent.prototype.nextURL = function () {
if (this.destroyed) {
return false;
}

var url = this.allAgents.hub.mountpoint,
test;

Expand Down Expand Up @@ -340,6 +349,10 @@ Agent.prototype.nextURL = function () {
* @param {Object} data Event payload.
*/
Agent.prototype.queueSocketEmit = function (event, data) {
if (this.destroyed) {
return false;
}

// Is anybody listening on the socketEmitter?
if (this.socketEmitter.children.length > 0) {
this.socketEmitter.emit(event, data);
Expand Down Expand Up @@ -371,20 +384,30 @@ Agent.prototype.available = function () {
};

/**
* TODO
*
* @method unload
* @method destroy
*/
Agent.prototype.unload = function () {
this.debug("disconnecting agent! stack:", (new Error()).stack);
Agent.prototype.destroy = function () {
if (this.destroyed) {
return false;
}

this.destroyed = true;

this.debug("disconnecting agent");
if (this.currentTest) {
this.currentTest.setExecuting(false);
}
this.stopPeriodicHealthCheck();
this.connected = false;
this.seen = new Date(0);
this.socketEmitter.end();
this.emit("disconnect");

this.target = null;
this.socketEmitter = null;
this.targetEmitter = null;
this.allAgents = null;

return true;
};

/**
Expand Down Expand Up @@ -457,13 +480,13 @@ Agent.prototype.getTimeoutMilliseconds = function () {
};

/**
* Check if this Agent should be unloaded,
* Check if this Agent should be destroyed,
* meaning that it has not pinged us for too long.
*
* @method shouldUnload
* @return {Boolean} True if the Agent is should unload, false otherwise.
* @method shouldDestroy
* @return {Boolean} True if the Agent is should be destroyed, false otherwise.
*/
Agent.prototype.shouldUnload = function () {
Agent.prototype.shouldDestroy = function () {
return this.unresponsivePings !== 0 && this.unresponsivePings > 3;
};

Expand Down
2 changes: 1 addition & 1 deletion lib/hub/all-agents.js
Expand Up @@ -136,7 +136,7 @@ AllAgents.prototype.removeAgent = function (id) {
var agent = this.agents[id];

if (agent) {
agent.unload();
agent.destroy();
delete this.agents[id];
}

Expand Down
16 changes: 12 additions & 4 deletions lib/hub/batch.js
Expand Up @@ -92,6 +92,14 @@ Batch.prototype.destroy = function () {
self.emit("end");
self.batchSession.emit("rpc.end");
self.batchSession.unbind();

self.allBatches = null;
self.batchSession = null;
self.runningTargets = null;
self.targets = null;
self.spec = null;
self.session = null;
self.testServer = null;
}

self.destroyed = true;
Expand All @@ -111,9 +119,6 @@ Batch.prototype.destroy = function () {
} else {
completer();
}

self.runningTargets = null;
self.targets = null;
};

Batch.prototype.completeTarget = function (target) {
Expand Down Expand Up @@ -318,6 +323,7 @@ Batch.prototype.handleFileRequest = function (server, agentId, filename) {
});

this.getFile(filename, function (err, buffer) {
var test;
if (err) {
if (agent) {
// If this file is in the current test batch,
Expand All @@ -330,7 +336,9 @@ Batch.prototype.handleFileRequest = function (server, agentId, filename) {
message: "Unable to serve the test: " + filename
});
if (agent.target) {
agent.target.tests.getByUrl(filename).setResults({});
test = agent.target.tests.getByUrl(filename);
test.setResults({});
test.setExecuting(false);
}
server.res.writeHead(302, {
"Location": agent.nextURL()
Expand Down
6 changes: 3 additions & 3 deletions lib/hub/index.js
Expand Up @@ -398,9 +398,9 @@ Hub.prototype._createRouter = function () {
id = opts[1],
agent = self.allAgents.getAgent(id);

//Keeps it safe from outside execution
if (agent && action === "unload") {
agent.unload();
// Keeps it safe from outside execution
if (agent && action === "destroy") {
agent.destroy();
this.res.writeHead(200, {
"content-type": "text/plain"
});
Expand Down
42 changes: 37 additions & 5 deletions lib/hub/target.js
Expand Up @@ -115,6 +115,8 @@ Target.prototype.setupEvents = function () {
var self = this;

self.agentEmitter.on("results", function (data) {
if (self.destroyed) { return; }

var test = self.tests.getByUrl(data.url);
if (!test.isExecuting()) { return; }
test.setResults(data);
Expand All @@ -123,6 +125,8 @@ Target.prototype.setupEvents = function () {
this.child.next();
});
self.agentEmitter.on("scriptError", function (data) {
if (self.destroyed) { return; }

var test = self.tests.getByUrl(data.url);
// TODO: Set results should include error message.
data.url = test.location;
Expand All @@ -133,11 +137,37 @@ Target.prototype.setupEvents = function () {
// TODO handle disconnects

self.agentEmitter.on("disconnect", function () {
if (self.destroyed) { return; }

self.emit("disconnect", this.child);
self.agentEmitter.remove(this.child);

if (self.agentEmitter.children.length === 0) {
self.emit("agentError", "All " + self.name + " browsers disconnected, giving up on this browser.");
self.complete();
}
});
};

/**
* @method complete
*/
Target.prototype.complete = function () {
if (this.destroyed) {
return;
}

this.destroyed = true;

this.emit("complete");

this.batchId = null;
this.tests = null;
this.batch = null;
this.agents = null;
this.agentEmitter = null;
};

/**
* Get the next Test.
*
Expand All @@ -150,17 +180,19 @@ Target.prototype.setupEvents = function () {
* @return {Test} Next test. May be a NullTest if we're done.
*/
Target.prototype.nextTest = function (agentId) {
if (this.tests.didComplete()) {
this.emit("complete");
this.batchId = null;
}
var didComplete = this.tests.didComplete(),
next = this.tests.next();

this.emit("progress", {
total: this.tests.totalSubmitted(),
current: this.tests.totalSubmitted() - this.tests.totalPending()
});

return this.tests.next();
if (didComplete) {
this.complete();
}

return next;
};

/**
Expand Down
20 changes: 10 additions & 10 deletions lib/hub/view/public/tempest.js
Expand Up @@ -248,13 +248,13 @@ YUI.add("tempest", function (Y, name) {
this.hub = new Y.HubClient(options);

/**
* Page unload event handler.
* Page destroy event handler.
*
* @property unloadHandler
* @property destroyHandler
* @private
* @type EventHandle|null
*/
this.unloadHandler = null;
this.destroyHandler = null;

/**
* Timeout for page navigation.
Expand Down Expand Up @@ -340,12 +340,12 @@ YUI.add("tempest", function (Y, name) {
};

/**
* @property unloadUrl
* @property destroyUrl
* @type String
*/
ATTRS.unloadUrl = {
ATTRS.destroyUrl = {
valueFn: function () {
return "/ping/unload/" + this.get("agentId");
return "/ping/destroy/" + this.get("agentId");
},
readOnly: true
};
Expand Down Expand Up @@ -419,7 +419,7 @@ YUI.add("tempest", function (Y, name) {
*/
proto.destroy = function () {
var self = this;
self.unloadHandler.detach();
self.destroyHandler.detach();
self.hub.destroy();
InjectedDriver.superclass.destroy.call(self);
self = null;
Expand Down Expand Up @@ -591,15 +591,15 @@ YUI.add("tempest", function (Y, name) {

self.debug("Attaching window event handlers.");

function unloadHandler() {
function destroyHandler() {
var img = new Image();
img.src = self.get("unloadUrl");
img.src = self.get("destroyUrl");
self.debug("Fetched " + img.src + ", destroying.");
self.destroy();
}

// Requires node-base.
self.unloadHandler = Y.on("unload", unloadHandler, win);
self.destroyHandler = Y.on("destroy", destroyHandler, win);
self.didSetupWindowHandlers = true;
};

Expand Down

0 comments on commit f35ab67

Please sign in to comment.