Skip to content

Commit

Permalink
process: use more direct sync I/O for stdio
Browse files Browse the repository at this point in the history
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: #18019
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
  • Loading branch information
addaleax committed Feb 26, 2018
1 parent f7e1402 commit cf0b95c
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 4 deletions.
24 changes: 24 additions & 0 deletions lib/internal/net.js
Original file line number Diff line number Diff line change
@@ -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) {
Expand All @@ -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')
};
16 changes: 12 additions & 4 deletions lib/net.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -213,20 +217,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;
Expand Down
3 changes: 3 additions & 0 deletions lib/tty.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
const util = require('util');
const net = require('net');
const { TTY, isTTY } = process.binding('tty_wrap');
const { makeSyncWrite } = require('internal/net');
const { inherits } = util;
const errnoException = util._errnoException;
const errors = require('internal/errors');
Expand Down Expand Up @@ -79,6 +80,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);
Expand Down

0 comments on commit cf0b95c

Please sign in to comment.