Skip to content

Commit cf0b95c

Browse files
committed
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: #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>
1 parent f7e1402 commit cf0b95c

File tree

3 files changed

+39
-4
lines changed

3 files changed

+39
-4
lines changed

lib/internal/net.js

+24
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
'use strict';
22

3+
const Buffer = require('buffer').Buffer;
4+
const { writeBuffer } = process.binding('fs');
5+
36
// Check that the port number is not NaN when coerced to a number,
47
// is an integer and that it falls within the legal range of port numbers.
58
function isLegalPort(port) {
@@ -9,7 +12,28 @@ function isLegalPort(port) {
912
return +port === (+port >>> 0) && port <= 0xFFFF;
1013
}
1114

15+
function makeSyncWrite(fd) {
16+
return function(chunk, enc, cb) {
17+
if (enc !== 'buffer')
18+
chunk = Buffer.from(chunk, enc);
19+
20+
this._bytesDispatched += chunk.length;
21+
22+
try {
23+
writeBuffer(fd, chunk, 0, chunk.length, null);
24+
} catch (ex) {
25+
// Legacy: net writes have .code === .errno, whereas writeBuffer gives the
26+
// raw errno number in .errno.
27+
if (typeof ex.code === 'string')
28+
ex.errno = ex.code;
29+
return cb(ex);
30+
}
31+
cb();
32+
};
33+
}
34+
1235
module.exports = {
1336
isLegalPort,
37+
makeSyncWrite,
1438
normalizedArgsSymbol: Symbol('normalizedArgs')
1539
};

lib/net.js

+12-4
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,11 @@ const stream = require('stream');
2626
const timers = require('timers');
2727
const util = require('util');
2828
const internalUtil = require('internal/util');
29-
const { isLegalPort, normalizedArgsSymbol } = require('internal/net');
29+
const {
30+
isLegalPort,
31+
normalizedArgsSymbol,
32+
makeSyncWrite
33+
} = require('internal/net');
3034
const assert = require('assert');
3135
const cares = process.binding('cares_wrap');
3236
const {
@@ -213,20 +217,24 @@ function Socket(options) {
213217
this._handle = options.handle; // private
214218
this[async_id_symbol] = getNewAsyncId(this._handle);
215219
} else if (options.fd !== undefined) {
216-
this._handle = createHandle(options.fd, false);
217-
this._handle.open(options.fd);
220+
const fd = options.fd;
221+
this._handle = createHandle(fd, false);
222+
this._handle.open(fd);
218223
this[async_id_symbol] = this._handle.getAsyncId();
219224
// options.fd can be string (since it is user-defined),
220225
// so changing this to === would be semver-major
221226
// See: https://github.com/nodejs/node/pull/11513
222227
// eslint-disable-next-line eqeqeq
223-
if ((options.fd == 1 || options.fd == 2) &&
228+
if ((fd == 1 || fd == 2) &&
224229
(this._handle instanceof Pipe) &&
225230
process.platform === 'win32') {
226231
// Make stdout and stderr blocking on Windows
227232
var err = this._handle.setBlocking(true);
228233
if (err)
229234
throw errnoException(err, 'setBlocking');
235+
236+
this._writev = null;
237+
this._write = makeSyncWrite(fd);
230238
}
231239
this.readable = options.readable !== false;
232240
this.writable = options.writable !== false;

lib/tty.js

+3
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
const util = require('util');
2525
const net = require('net');
2626
const { TTY, isTTY } = process.binding('tty_wrap');
27+
const { makeSyncWrite } = require('internal/net');
2728
const { inherits } = util;
2829
const errnoException = util._errnoException;
2930
const errors = require('internal/errors');
@@ -79,6 +80,8 @@ function WriteStream(fd) {
7980
// even though it was originally intended to change in v1.0.2 (Libuv 1.2.1).
8081
// Ref: https://github.com/nodejs/node/pull/1771#issuecomment-119351671
8182
this._handle.setBlocking(true);
83+
this._writev = null;
84+
this._write = makeSyncWrite(fd);
8285

8386
var winSize = new Array(2);
8487
var err = this._handle.getWindowSize(winSize);

0 commit comments

Comments
 (0)