Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

Commit

Permalink
Fix a race condition or two in net.js
Browse files Browse the repository at this point in the history
When making a TCP connection, readyState returns 'opening' while resolving
the host. However between the resolving period and the establishing a
connection period, it would return 'closed'. This fixes it.

This change also ensures that the socket is closed before the 'end' event is
emitted in the case that the socket was previously shutdown.
  • Loading branch information
ry committed Apr 12, 2010
1 parent 62d9852 commit 4681e34
Showing 1 changed file with 18 additions and 13 deletions.
31 changes: 18 additions & 13 deletions lib/net.js
Expand Up @@ -308,13 +308,15 @@ function initStream (self) {
//debug('bytesRead ' + bytesRead + '\n');

if (bytesRead === 0) {
// EOF
self.readable = false;
self._readWatcher.stop();

if (!self.writable) self.destroy();
// Note: 'close' not emitted until nextTick.

if (self._events && self._events['end']) self.emit('end');
if (self.onend) self.onend();

if (!self.writable) self.destroy();
} else if (bytesRead > 0) {

timeout.active(self);
Expand Down Expand Up @@ -383,15 +385,19 @@ exports.createConnection = function (port, host) {

Object.defineProperty(Stream.prototype, 'readyState', {
get: function () {
if (this._resolving) {
if (this._connecting) {
return 'opening';
} else if (this.readable && this.writable) {
assert(typeof this.fd == 'number');
return 'open';
} else if (this.readable && !this.writable){
assert(typeof this.fd == 'number');
return 'readOnly';
} else if (!this.readable && this.writable){
assert(typeof this.fd == 'number');
return 'writeOnly';
} else {
assert(typeof this.fd != 'number');
return 'closed';
}
}
Expand Down Expand Up @@ -580,16 +586,16 @@ function doConnect (socket, port, host) {
// socketError() if there isn't an error, we're connected. AFAIK this a
// platform independent way determining when a non-blocking connection
// is established, but I have only seen it documented in the Linux
// Manual Page connect(2) under the error code EINPROGRESS.
// Manual Page connect(2) under the error code EINPROGRESS.
socket._writeWatcher.set(socket.fd, false, true);
socket._writeWatcher.start();
socket._writeWatcher.callback = function () {
var errno = socketError(socket.fd);
if (errno == 0) {
// connection established
socket._connecting = false;
socket.resume();
socket.readable = true;
socket.writable = true;
socket.readable = socket.writable = true;
socket._writeWatcher.callback = _doFlush;
socket.emit('connect');
} else if (errno != EINPROGRESS) {
Expand All @@ -611,27 +617,24 @@ Stream.prototype.connect = function () {

timeout.active(socket);

var port = parseInt(arguments[0]);
self._connecting = true; // set false in doConnect

if (port >= 0) {
//debug('new fd = ' + self.fd);
// TODO dns resolution on arguments[1]
if (parseInt(arguments[0]) >= 0) {
// TCP
var port = arguments[0];
self._resolving = true;
dns.lookup(arguments[1], function (err, ip, addressType) {
if (err) {
self.emit('error', err);
} else {
self.type = addressType == 4 ? 'tcp4' : 'tcp6';
self.fd = socket(self.type);
self._resolving = false;
doConnect(self, port, ip);
}
});
} else {
// UNIX
self.fd = socket('unix');
self.type = 'unix';
// TODO check if sockfile exists?
doConnect(self, arguments[0]);
}
};
Expand Down Expand Up @@ -683,6 +686,8 @@ Stream.prototype.destroy = function (exception) {
// but lots of code assumes this._writeQueue is always an array.
this._writeQueue = [];

this.readable = this.writable = false;

if (this._writeWatcher) {
this._writeWatcher.stop();
ioWatchers.free(this._writeWatcher);
Expand Down

0 comments on commit 4681e34

Please sign in to comment.