Skip to content

Commit

Permalink
fs: make SyncWriteStream inherit from Writable
Browse files Browse the repository at this point in the history
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: #8828
PR-URL: #8830
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Jeremiah Senkpiel <[email protected]>

(backport info)
Refs: #9030
  • Loading branch information
addaleax authored and Fishrock123 committed Oct 11, 2016
1 parent 5e6be7b commit 688abac
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 41 deletions.
58 changes: 17 additions & 41 deletions lib/fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
};
40 changes: 40 additions & 0 deletions test/parallel/test-fs-syncwritestream.js
Original file line number Diff line number Diff line change
@@ -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 }
]);
}));

0 comments on commit 688abac

Please sign in to comment.