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: nodejs#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 Jan 14, 2018
1 parent c84582c commit 8b751f7
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 @@ -220,20 +224,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 @@ -91,6 +92,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 8b751f7

Please sign in to comment.