stream: call write cb before finish event

Since 049903e, an end callback could be called before a write
callback if end() is called before the write is done. This patch
resolves the issue.

In collaboration with @gne

Fixes felixge/node-formidable#209
Fixes #5215

diff --git a/lib/_stream_writable.js b/lib/_stream_writable.js
index 346f2cc96ab4f1519522a7f19c2bf0dde5e92239..c060e015a05489878c8e4f83df7d0c24b14cd011 100644 (file)
--- a/lib/_stream_writable.js
+++ b/lib/_stream_writable.js
@@ -240,7 +240,8 @@ function onwrite(stream, er) {
   if (er)
     onwriteError(stream, state, sync, er, cb);
   else {
-    var finished = finishMaybe(stream, state);
+    // Check if we're actually ready to finish, but don't emit yet
+    var finished = needFinish(stream, state);
 
     if (!finished && !state.bufferProcessing && state.buffer.length)
       clearBuffer(stream, state);
@@ -259,6 +260,8 @@ function afterWrite(stream, state, finished, cb) {
   if (!finished)
     onwriteDrain(stream, state);
   cb();
+  if (finished)
+    finishMaybe(stream, state);
 }
 
 // Must force callback to be called on nextTick, so that we don't
@@ -326,15 +329,21 @@ Writable.prototype.end = function(chunk, encoding, cb) {
     endWritable(this, state, cb);
 };
 
+
+function needFinish(stream, state) {
+  return (state.ending &&
+          state.length === 0 &&
+          !state.finished &&
+          !state.writing);
+}
+
 function finishMaybe(stream, state) {
-  if (state.ending &&
-      state.length === 0 &&
-      !state.finished &&
-      !state.writing) {
+  var need = needFinish(stream, state);
+  if (need) {
     state.finished = true;
     stream.emit('finish');
   }
-  return state.finished;
+  return need;
 }
 
 function endWritable(stream, state, cb) {

diff --git a/test/simple/test-stream2-writable.js b/test/simple/test-stream2-writable.js
index bdb7df2d04db1f69b8b6e70d04f15c5b9685c788..e0f384cb2ad639d80042cb77bbfa55aa3d9de1a3 100644 (file)
--- a/test/simple/test-stream2-writable.js
+++ b/test/simple/test-stream2-writable.js
@@ -275,6 +275,18 @@ test('end callback after .write() call', function (t) {
   });
 });
 
+test('end callback called after write callback', function (t) {
+  var tw = new TestWriter();
+  var writeCalledback = false;
+  tw.write(new Buffer('hello world'),  function() {
+    writeCalledback = true;
+  });
+  tw.end(function () {
+    t.equal(writeCalledback, true);
+    t.end();
+  });
+});
+
 test('encoding should be ignored for buffers', function(t) {
   var tw = new W();
   var hex = '018b5e9a8f6236ffe30e31baf80d2cf6eb';
@@ -346,3 +358,20 @@ test('dont end while writing', function(t) {
   w.write(Buffer(0));
   w.end();
 });
+
+test('finish does not come before write cb', function(t) {
+  var w = new W();
+  var writeCb = false;
+  w._write = function(chunk, e, cb) {
+    setTimeout(function() {
+      writeCb = true;
+      cb();
+    }, 10);
+  };
+  w.on('finish', function() {
+    assert(writeCb);
+    t.end();
+  });
+  w.write(Buffer(0));
+  w.end();
+});