From b1fc7745f2c7f279d388d8c88781df776b740259 Mon Sep 17 00:00:00 2001 From: Matteo Collina Date: Tue, 7 Feb 2017 19:38:33 +0100 Subject: [PATCH] fs: avoid emitting error EBADF for double close Changed the logic in fs.ReadStream and fs.WriteStream so that close always calls the prototype method rather than the internal event listener. Fixes: https://github.com/nodejs/node/issues/2950 PR-URL: https://github.com/nodejs/node/pull/11225 Reviewed-By: Colin Ihrig Reviewed-By: James M Snell Reviewed-By: Evan Lucas --- lib/fs.js | 25 +++++++++---------- .../test-fs-read-stream-double-close.js | 11 ++++++++ .../test-fs-write-stream-double-close.js | 14 +++++++++++ 3 files changed, 37 insertions(+), 13 deletions(-) create mode 100644 test/parallel/test-fs-read-stream-double-close.js create mode 100644 test/parallel/test-fs-write-stream-double-close.js diff --git a/lib/fs.js b/lib/fs.js index fae2857e3fc759..c05e9e592d0c7c 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -1855,28 +1855,27 @@ ReadStream.prototype.destroy = function() { ReadStream.prototype.close = function(cb) { - var self = this; if (cb) this.once('close', cb); + if (this.closed || typeof this.fd !== 'number') { if (typeof this.fd !== 'number') { - this.once('open', close); + this.once('open', this.close.bind(this, null)); return; } return process.nextTick(() => this.emit('close')); } + this.closed = true; - close(); - - function close(fd) { - fs.close(fd || self.fd, function(er) { - if (er) - self.emit('error', er); - else - self.emit('close'); - }); - self.fd = null; - } + + fs.close(this.fd, (er) => { + if (er) + this.emit('error', er); + else + this.emit('close'); + }); + + this.fd = null; }; diff --git a/test/parallel/test-fs-read-stream-double-close.js b/test/parallel/test-fs-read-stream-double-close.js new file mode 100644 index 00000000000000..e18bc6b4fb17d1 --- /dev/null +++ b/test/parallel/test-fs-read-stream-double-close.js @@ -0,0 +1,11 @@ +'use strict'; + +const common = require('../common'); +const fs = require('fs'); + +const s = fs.createReadStream(__filename); + +s.close(common.mustCall(noop)); +s.close(common.mustCall(noop)); + +function noop() {} diff --git a/test/parallel/test-fs-write-stream-double-close.js b/test/parallel/test-fs-write-stream-double-close.js new file mode 100644 index 00000000000000..935d58eaef9cf2 --- /dev/null +++ b/test/parallel/test-fs-write-stream-double-close.js @@ -0,0 +1,14 @@ +'use strict'; + +const common = require('../common'); +const fs = require('fs'); +const path = require('path'); + +common.refreshTmpDir(); + +const s = fs.createWriteStream(path.join(common.tmpDir, 'rw')); + +s.close(common.mustCall(noop)); +s.close(common.mustCall(noop)); + +function noop() {}