don't crash when queued write fails

nodejs · Mar 23, 2012 · 0dcc433 · 0dcc433
1 parent d227084
commit 0dcc433
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 26 deletions.
diff --git a/lib/net.js b/lib/net.js
@@ -84,6 +84,7 @@ function initSocketHandle(self) {
   self._flags = 0;
   self._connectQueueSize = 0;
   self.destroyed = false;
+  self.errorEmitted = false;
   self.bytesRead = 0;
   self.bytesWritten = 0;
 
@@ -244,7 +245,7 @@ Socket.prototype.end = function(data, encoding) {
     var shutdownReq = this._handle.shutdown();
 
     if (!shutdownReq) {
-      this.destroy(errnoException(errno, 'shutdown'));
+      this._destroy(errnoException(errno, 'shutdown'));
       return false;
     }
 
@@ -267,7 +268,7 @@ function afterShutdown(status, handle, req) {
   }
 
   if (self._flags & FLAG_GOT_EOF || !self.readable) {
-    self.destroy();
+    self._destroy();
   } else {
   }
 }
@@ -278,7 +279,7 @@ Socket.prototype.destroySoon = function() {
   this._flags |= FLAG_DESTROY_SOON;
 
   if (this._pendingWriteReqs == 0) {
-    this.destroy();
+    this._destroy();
   }
 };
 
@@ -290,11 +291,24 @@ Socket.prototype._connectQueueCleanUp = function(exception) {
 };
 
 
-Socket.prototype.destroy = function(exception) {
-  if (this.destroyed) return;
-
+Socket.prototype._destroy = function(exception, cb) {
   var self = this;
 
+  function fireErrorCallbacks() {
+    if (cb) cb(exception);
+    if (exception && !self.errorEmitted) {
+      process.nextTick(function() {
+        self.emit('error', exception);
+      });
+      self.errorEmitted = true;
+    }
+  };
+
+  if (this.destroyed) {
+    fireErrorCallbacks();
+    return;
+  }
+
   self._connectQueueCleanUp();
 
   debug('destroy');
@@ -315,15 +329,21 @@ Socket.prototype.destroy = function(exception) {
     this._handle = null;
   }
 
+  fireErrorCallbacks();
+
   process.nextTick(function() {
-    if (exception) self.emit('error', exception);
     self.emit('close', exception ? true : false);
   });
 
   this.destroyed = true;
 };
 
 
+Socket.prototype.destroy = function(exception) {
+  this._destroy(exception);
+}
+
+
 function onread(buffer, offset, length) {
   var handle = this;
   var self = handle.socket;
@@ -362,17 +382,17 @@ function onread(buffer, offset, length) {
 
     // We call destroy() before end(). 'close' not emitted until nextTick so
     // the 'end' event will come first as required.
-    if (!self.writable) self.destroy();
+    if (!self.writable) self._destroy();
 
     if (!self.allowHalfOpen) self.end();
     if (self._events && self._events['end']) self.emit('end');
     if (self.onend) self.onend();
   } else {
     // Error
     if (errno == 'ECONNRESET') {
-      self.destroy();
+      self._destroy();
     } else {
-      self.destroy(errnoException(errno, 'read'));
+      self._destroy(errnoException(errno, 'read'));
     }
   }
 }
@@ -450,13 +470,16 @@ Socket.prototype.write = function(data, arg1, arg2) {
 Socket.prototype._write = function(data, encoding, cb) {
   timers.active(this);
 
-  if (!this._handle) throw new Error('This socket is closed.');
+  if (!this._handle) {
+    this._destroy(new Error('This socket is closed.'), cb);
+    return false;
+  }
 
   // `encoding` is unused right now, `data` is always a buffer.
   var writeReq = this._handle.write(data);
 
   if (!writeReq) {
-    this.destroy(errnoException(errno, 'write'));
+    this._destroy(errnoException(errno, 'write'), cb);
     return false;
   }
 
@@ -477,7 +500,7 @@ function afterWrite(status, handle, req, buffer) {
   }
 
   if (status) {
-    self.destroy(errnoException(errno, 'write'));
+    self._destroy(errnoException(errno, 'write'), req.cb);
     return;
   }
 
@@ -494,7 +517,7 @@ function afterWrite(status, handle, req, buffer) {
   if (req.cb) req.cb();
 
   if (self._pendingWriteReqs == 0 && self._flags & FLAG_DESTROY_SOON) {
-    self.destroy();
+    self._destroy();
   }
 }
 
@@ -522,7 +545,7 @@ function connect(self, address, port, addressType) {
   if (connectReq !== null) {
     connectReq.oncomplete = afterConnect;
   } else {
-    self.destroy(errnoException(errno, 'connect'));
+    self._destroy(errnoException(errno, 'connect'));
   }
 }
 
@@ -570,7 +593,7 @@ Socket.prototype.connect = function(port /* [host], [cb] */) {
         // error event to the next tick.
         process.nextTick(function() {
           self.emit('error', err);
-          self.destroy();
+          self._destroy();
         });
       } else {
         timers.active(self);
@@ -619,8 +642,9 @@ function afterConnect(status, handle, req, readable, writable) {
 
     if (self._connectQueue) {
       debug('Drain the connect queue');
-      for (var i = 0; i < self._connectQueue.length; i++) {
-        self._write.apply(self, self._connectQueue[i]);
+      var connectQueue = self._connectQueue;
+      for (var i = 0; i < connectQueue.length; i++) {
+        self._write.apply(self, connectQueue[i]);
       }
       self._connectQueueCleanUp();
     }
@@ -634,7 +658,7 @@ function afterConnect(status, handle, req, readable, writable) {
     }
   } else {
     self._connectQueueCleanUp();
-    self.destroy(errnoException(errno, 'connect'));
+    self._destroy(errnoException(errno, 'connect'));
   }
 }
 

diff --git a/test/simple/test-net-write-after-close.js b/test/simple/test-net-write-after-close.js
@@ -24,21 +24,23 @@ var assert = require('assert');
 var net = require('net');
 
 var gotError = false;
+var gotWriteCB = false;
 
 process.on('exit', function() {
   assert(gotError);
+  assert(gotWriteCB);
 });
 
 var server = net.createServer(function(socket) {
-  setTimeout(function() {
-    assert.throws(
-      function() {
-        socket.write('test');
-      },
-      /This socket is closed/
-    );
+  socket.on('error', function(error) {
     server.close();
     gotError = true;
+  });
+
+  setTimeout(function() {
+    socket.write('test', function(e) {
+      gotWriteCB = true;
+    });
   }, 250);
 });