From 55e9c3aedb804017c50c4759d21ed8f782c1b144 Mon Sep 17 00:00:00 2001
From: Anna Henningsen <anna@addaleax.net>
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 <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
---
 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 }
+  ]);
+}));