From 4681e34c1e74e96f3027e88523dadf6b27a611c3 Mon Sep 17 00:00:00 2001
From: Ryan Dahl <ry@tinyclouds.org>
Date: Mon, 12 Apr 2010 12:34:24 -0700
Subject: [PATCH] Fix a race condition or two in net.js

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.
---
 lib/net.js | 31 ++++++++++++++++++-------------
 1 file changed, 18 insertions(+), 13 deletions(-)

diff --git a/lib/net.js b/lib/net.js
index f3461c5..bf7caf6 100644
--- a/lib/net.js
+++ b/lib/net.js
@@ -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);
@@ -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';
     }
   }
@@ -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) {
@@ -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]);
   }
 };
@@ -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);
-- 
2.7.4