Skip to content

Commit

Permalink
http: fix memory leak when socket dns lookup occurs error (#348)
Browse files Browse the repository at this point in the history
  • Loading branch information
qile222 authored and yorkie committed Sep 25, 2018
1 parent b45d0f0 commit 535cd07
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 9 deletions.
5 changes: 0 additions & 5 deletions src/js/http_client.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ function setupConnection(req, socket) {
socket.on('data', socketOnData);
socket.on('end', socketOnEnd);
socket.on('close', socketOnClose);
socket.on('lookup', socketOnLookup);

// socket emitted when a socket is assigned to req
process.nextTick(function() {
Expand Down Expand Up @@ -166,10 +165,6 @@ function socketOnError(err) {
emitError(this, err);
}

function socketOnLookup(err, ip, family) {
emitError(this, err);
}

function socketOnData(d) {
var socket = this;
var req = this._httpMessage;
Expand Down
5 changes: 1 addition & 4 deletions src/js/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,10 +141,7 @@ Socket.prototype.connect = function() {
self.emit('lookup', err, ip, family);

if (err) {
process.nextTick(function() {
self.emit('error', err);
self.destroy();
});
emitError(self, err);
} else {
resetSocketTimeout(self);
process.nextTick(function() {
Expand Down
19 changes: 19 additions & 0 deletions test/run_pass/test_net_http_dns_error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
var assert = require('assert');
var http = require('http');
var common = require('../common');

var host = 'www.unable-to-resolved-host.rokid.com';
var req = http.request({ host: host });

req.socket.on('finish', common.mustCall(() => {
process.nextTick(() => {
var destroyed = req.socket._socketState.destroyed;
console.log(`socket is destroyed ${host}:`, destroyed);
assert.equal(destroyed, true);
});
}));

// handle error event to prevent the process throw an error
req.on('error', common.mustCall((err) => {
console.log('request error', err.message);
}));
1 change: 1 addition & 0 deletions test/testsets.json
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
{ "name": "test_net_connect.js" },
{ "name": "test_net_end.js" },
{ "name": "test_net_headers.js" },
{ "name": "test_net_http_dns_error.js" },
{ "name": "test_net_http_get.js" },
{ "name": "test_net_http_response_twice.js" },
{ "name": "test_net_http_request_response.js", "skip": ["nuttx"], "reason": "not implemented for nuttx" },
Expand Down

0 comments on commit 535cd07

Please sign in to comment.