From 688abac7b219ce05dd64101f9d543b9f162f88d2 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Wed, 28 Sep 2016 16:56:38 +0200 Subject: [PATCH] fs: make `SyncWriteStream` inherit from `Writable` Make the internal `SyncWriteStream` a proper `stream.Writable` subclass. This allows for quite a bit of simplification, since `SyncWriteStream` predates the streams2/streams3 implementations. Fixes: https://github.com/nodejs/node/issues/8828 PR-URL: https://github.com/nodejs/node/pull/8830 Reviewed-By: Matteo Collina Reviewed-By: James M Snell Reviewed-By: Jeremiah Senkpiel (backport info) Refs: https://github.com/nodejs/node/pull/9030 --- lib/fs.js | 58 +++++++----------------- test/parallel/test-fs-syncwritestream.js | 40 ++++++++++++++++ 2 files changed, 57 insertions(+), 41 deletions(-) create mode 100644 test/parallel/test-fs-syncwritestream.js diff --git a/lib/fs.js b/lib/fs.js index ba8f0825d411a9..cb99fcf8211391 100644 --- a/lib/fs.js +++ b/lib/fs.js @@ -2196,17 +2196,18 @@ WriteStream.prototype.destroySoon = WriteStream.prototype.end; // SyncWriteStream is internal. DO NOT USE. // Temporary hack for process.stdout and process.stderr when piped to files. function SyncWriteStream(fd, options) { - Stream.call(this); + Writable.call(this); options = options || {}; this.fd = fd; - this.writable = true; this.readable = false; this.autoClose = options.autoClose === undefined ? true : options.autoClose; + + this.on('end', () => this._destroy()); } -util.inherits(SyncWriteStream, Stream); +util.inherits(SyncWriteStream, Writable); // Export @@ -2216,51 +2217,26 @@ Object.defineProperty(fs, 'SyncWriteStream', { value: SyncWriteStream }); -SyncWriteStream.prototype.write = function(data, arg1, arg2) { - var encoding, cb; - - // parse arguments - if (arg1) { - if (typeof arg1 === 'string') { - encoding = arg1; - cb = arg2; - } else if (typeof arg1 === 'function') { - cb = arg1; - } else { - throw new Error('Bad arguments'); - } - } - assertEncoding(encoding); - - // Change strings to buffers. SLOW - if (typeof data === 'string') { - data = Buffer.from(data, encoding); - } - - fs.writeSync(this.fd, data, 0, data.length); - - if (cb) { - process.nextTick(cb); - } - +SyncWriteStream.prototype._write = function(chunk, encoding, cb) { + fs.writeSync(this.fd, chunk, 0, chunk.length); + cb(); return true; }; +SyncWriteStream.prototype._destroy = function() { + if (this.fd === null) // already destroy()ed + return; -SyncWriteStream.prototype.end = function(data, arg1, arg2) { - if (data) { - this.write(data, arg1, arg2); - } - this.destroy(); -}; - - -SyncWriteStream.prototype.destroy = function() { if (this.autoClose) fs.closeSync(this.fd); + this.fd = null; - this.emit('close'); return true; }; -SyncWriteStream.prototype.destroySoon = SyncWriteStream.prototype.destroy; +SyncWriteStream.prototype.destroySoon = +SyncWriteStream.prototype.destroy = function() { + this._destroy(); + this.emit('close'); + return true; +}; diff --git a/test/parallel/test-fs-syncwritestream.js b/test/parallel/test-fs-syncwritestream.js new file mode 100644 index 00000000000000..236c412c45b543 --- /dev/null +++ b/test/parallel/test-fs-syncwritestream.js @@ -0,0 +1,40 @@ +'use strict'; +const common = require('../common'); +const assert = require('assert'); +const spawn = require('child_process').spawn; +const stream = require('stream'); +const fs = require('fs'); +const path = require('path'); + +// require('internal/fs').SyncWriteStream is used as a stdio implementation +// when stdout/stderr point to files. + +if (process.argv[2] === 'child') { + // Note: Calling console.log() is part of this test as it exercises the + // SyncWriteStream#_write() code path. + console.log(JSON.stringify([process.stdout, process.stderr].map((stdio) => ({ + instance: stdio instanceof stream.Writable, + readable: stdio.readable, + writable: stdio.writable, + })))); + + return; +} + +common.refreshTmpDir(); + +const filename = path.join(common.tmpDir, 'stdout'); +const stdoutFd = fs.openSync(filename, 'w'); + +const proc = spawn(process.execPath, [__filename, 'child'], { + stdio: ['inherit', stdoutFd, stdoutFd ] +}); + +proc.on('close', common.mustCall(() => { + fs.closeSync(stdoutFd); + + assert.deepStrictEqual(JSON.parse(fs.readFileSync(filename, 'utf8')), [ + { instance: true, readable: false, writable: true }, + { instance: true, readable: false, writable: true } + ]); +}));