diff --git a/lib/fs.js b/lib/fs.js index eb781b0add8588..d4988eb649f2e1 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -2002,6 +2002,7 @@ function ReadStream(path, options) { this.autoClose = options.autoClose === undefined ? true : options.autoClose; this.pos = undefined; this.bytesRead = 0; + this.closed = false; if (this.start !== undefined) { if (typeof this.start !== 'number') { @@ -2115,20 +2116,12 @@ ReadStream.prototype._read = function(n) { }; ReadStream.prototype._destroy = function(err, cb) { - if (this.closed || typeof this.fd !== 'number') { - if (typeof this.fd !== 'number') { - this.once('open', closeFsStream.bind(null, this, cb, err)); - return; - } - - return process.nextTick(() => { - cb(err); - this.emit('close'); - }); + const isOpen = typeof this.fd !== 'number'; + if (isOpen) { + this.once('open', closeFsStream.bind(null, this, cb, err)); + return; } - this.closed = true; - closeFsStream(this, cb); this.fd = null; }; @@ -2137,6 +2130,7 @@ function closeFsStream(stream, cb, err) { fs.close(stream.fd, (er) => { er = er || err; cb(er); + stream.closed = true; if (!er) stream.emit('close'); }); @@ -2169,6 +2163,7 @@ function WriteStream(path, options) { this.autoClose = options.autoClose === undefined ? true : !!options.autoClose; this.pos = undefined; this.bytesWritten = 0; + this.closed = false; if (this.start !== undefined) { if (typeof this.start !== 'number') { @@ -2294,19 +2289,24 @@ WriteStream.prototype._writev = function(data, cb) { WriteStream.prototype._destroy = ReadStream.prototype._destroy; WriteStream.prototype.close = function(cb) { - if (this._writableState.ending) { - this.on('close', cb); - return; + if (cb) { + if (this.closed) { + process.nextTick(cb); + return; + } else { + this.on('close', cb); + } } - if (this._writableState.ended) { - process.nextTick(cb); - return; + // If we are not autoClosing, we should call + // destroy on 'finish'. + if (!this.autoClose) { + this.on('finish', this.destroy.bind(this)); } // we use end() instead of destroy() because of // https://github.com/nodejs/node/issues/2006 - this.end(cb); + this.end(); }; // There is no shutdown() for files. diff --git a/test/parallel/test-fs-write-stream-autoclose-option.js b/test/parallel/test-fs-write-stream-autoclose-option.js index df73d18b44210d..cc22ef660a15b8 100644 --- a/test/parallel/test-fs-write-stream-autoclose-option.js +++ b/test/parallel/test-fs-write-stream-autoclose-option.js @@ -10,8 +10,9 @@ let stream = fs.createWriteStream(file, { flags: 'w+', autoClose: false }); stream.write('Test1'); stream.end(); stream.on('finish', common.mustCall(function() { + stream.on('close', common.mustNotCall()); process.nextTick(common.mustCall(function() { - assert.strictEqual(stream.closed, undefined); + assert.strictEqual(stream.closed, false); assert.notStrictEqual(stream.fd, null); next(); })); @@ -23,9 +24,12 @@ function next() { stream.write('Test2'); stream.end(); stream.on('finish', common.mustCall(function() { - assert.strictEqual(stream.closed, true); + assert.strictEqual(stream.closed, false); assert.strictEqual(stream.fd, null); - process.nextTick(common.mustCall(next2)); + stream.on('close', common.mustCall(function() { + assert.strictEqual(stream.closed, true); + process.nextTick(next2); + })); })); } @@ -44,9 +48,10 @@ function next3() { stream.write('Test3'); stream.end(); stream.on('finish', common.mustCall(function() { - process.nextTick(common.mustCall(function() { + assert.strictEqual(stream.closed, false); + assert.strictEqual(stream.fd, null); + stream.on('close', common.mustCall(function() { assert.strictEqual(stream.closed, true); - assert.strictEqual(stream.fd, null); })); })); } diff --git a/test/parallel/test-fs-write-stream-close-without-callback.js b/test/parallel/test-fs-write-stream-close-without-callback.js new file mode 100644 index 00000000000000..95b52fe7af7082 --- /dev/null +++ b/test/parallel/test-fs-write-stream-close-without-callback.js @@ -0,0 +1,12 @@ +'use strict'; + +const common = require('../common'); +const fs = require('fs'); +const path = require('path'); + +common.refreshTmpDir(); + +const s = fs.createWriteStream(path.join(common.tmpDir, 'nocallback')); + +s.end('hello world'); +s.close(); diff --git a/test/parallel/test-fs-write-stream-double-close.js b/test/parallel/test-fs-write-stream-double-close.js index c73c9c7d6ad9ae..10ce9077a0afd4 100644 --- a/test/parallel/test-fs-write-stream-double-close.js +++ b/test/parallel/test-fs-write-stream-double-close.js @@ -1,12 +1,45 @@ 'use strict'; const common = require('../common'); +const assert = require('assert'); const fs = require('fs'); const path = require('path'); common.refreshTmpDir(); -const s = fs.createWriteStream(path.join(common.tmpDir, 'rw')); +{ + const s = fs.createWriteStream(path.join(common.tmpDir, 'rw')); -s.close(common.mustCall()); -s.close(common.mustCall()); + s.close(common.mustCall()); + s.close(common.mustCall()); +} + +{ + const s = fs.createWriteStream(path.join(common.tmpDir, 'rw2')); + + let emits = 0; + s.on('close', () => { + emits++; + }); + + s.close(common.mustCall(() => { + assert.strictEqual(emits, 1); + s.close(common.mustCall(() => { + assert.strictEqual(emits, 1); + })); + process.nextTick(() => { + s.close(common.mustCall(() => { + assert.strictEqual(emits, 1); + })); + }); + })); +} + +{ + const s = fs.createWriteStream(path.join(common.tmpDir, 'rw'), { + autoClose: false + }); + + s.close(common.mustCall()); + s.close(common.mustCall()); +}