From 2ec981b0781c4b387c729304965f21e787dbd9e0 Mon Sep 17 00:00:00 2001 From: Anna Henningsen Date: Sun, 31 Dec 2017 00:27:56 +0100 Subject: [PATCH] process: use more direct sync I/O for stdio This avoids routing writes through the full LibuvStreamWrap write machinery. In particular, it enables the next commit, because otherwise the callback passed to `_write()` would not be called synchronously for pipes on Windows (because the latter does not support `uv_try_write()`, even for blocking I/O). PR-URL: https://github.com/nodejs/node/pull/18019 Reviewed-By: Anatoli Papirovski Reviewed-By: Ben Noordhuis Reviewed-By: James M Snell Reviewed-By: Colin Ihrig --- lib/internal/net.js | 24 ++++++++++++++++++++++++ lib/net.js | 16 ++++++++++++---- lib/tty.js | 3 +++ 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/lib/internal/net.js b/lib/internal/net.js index d4558cfca8e214..847539d576906d 100644 --- a/lib/internal/net.js +++ b/lib/internal/net.js @@ -1,5 +1,8 @@ 'use strict'; +const Buffer = require('buffer').Buffer; +const { writeBuffer } = process.binding('fs'); + // Check that the port number is not NaN when coerced to a number, // is an integer and that it falls within the legal range of port numbers. function isLegalPort(port) { @@ -9,7 +12,28 @@ function isLegalPort(port) { return +port === (+port >>> 0) && port <= 0xFFFF; } +function makeSyncWrite(fd) { + return function(chunk, enc, cb) { + if (enc !== 'buffer') + chunk = Buffer.from(chunk, enc); + + this._bytesDispatched += chunk.length; + + try { + writeBuffer(fd, chunk, 0, chunk.length, null); + } catch (ex) { + // Legacy: net writes have .code === .errno, whereas writeBuffer gives the + // raw errno number in .errno. + if (typeof ex.code === 'string') + ex.errno = ex.code; + return cb(ex); + } + cb(); + }; +} + module.exports = { isLegalPort, + makeSyncWrite, normalizedArgsSymbol: Symbol('normalizedArgs') }; diff --git a/lib/net.js b/lib/net.js index 4c84c49430e6cd..98bcbc3b7c40f3 100644 --- a/lib/net.js +++ b/lib/net.js @@ -26,7 +26,11 @@ const stream = require('stream'); const timers = require('timers'); const util = require('util'); const internalUtil = require('internal/util'); -const { isLegalPort, normalizedArgsSymbol } = require('internal/net'); +const { + isLegalPort, + normalizedArgsSymbol, + makeSyncWrite +} = require('internal/net'); const assert = require('assert'); const cares = process.binding('cares_wrap'); const uv = process.binding('uv'); @@ -206,20 +210,24 @@ function Socket(options) { this._handle = options.handle; // private this[async_id_symbol] = getNewAsyncId(this._handle); } else if (options.fd !== undefined) { - this._handle = createHandle(options.fd, false); - this._handle.open(options.fd); + const fd = options.fd; + this._handle = createHandle(fd, false); + this._handle.open(fd); this[async_id_symbol] = this._handle.getAsyncId(); // options.fd can be string (since it is user-defined), // so changing this to === would be semver-major // See: https://github.com/nodejs/node/pull/11513 // eslint-disable-next-line eqeqeq - if ((options.fd == 1 || options.fd == 2) && + if ((fd == 1 || fd == 2) && (this._handle instanceof Pipe) && process.platform === 'win32') { // Make stdout and stderr blocking on Windows var err = this._handle.setBlocking(true); if (err) throw errnoException(err, 'setBlocking'); + + this._writev = null; + this._write = makeSyncWrite(fd); } this.readable = options.readable !== false; this.writable = options.writable !== false; diff --git a/lib/tty.js b/lib/tty.js index 29440d6d96e3d5..d5e6ba12df2383 100644 --- a/lib/tty.js +++ b/lib/tty.js @@ -24,6 +24,7 @@ const { inherits, _extend } = require('util'); const net = require('net'); const { TTY, isTTY } = process.binding('tty_wrap'); +const { makeSyncWrite } = require('internal/net'); const errors = require('internal/errors'); const readline = require('readline'); @@ -77,6 +78,8 @@ function WriteStream(fd) { // even though it was originally intended to change in v1.0.2 (Libuv 1.2.1). // Ref: https://github.com/nodejs/node/pull/1771#issuecomment-119351671 this._handle.setBlocking(true); + this._writev = null; + this._write = makeSyncWrite(fd); var winSize = new Array(2); var err = this._handle.getWindowSize(winSize);