Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

process: process.stdin getter sets O_NONBLOCK flag in open file description #42826

Closed
0x2b3bfa0 opened this issue Apr 22, 2022 · 9 comments
Closed

Comments

@0x2b3bfa0
Copy link

0x2b3bfa0 commented Apr 22, 2022

Version

v18.0.0

Platform

Linux codespaces-151d18 5.4.0-1074-azure #77~18.04.1-Ubuntu SMP Wed Mar 30 15:36:02 UTC 2022 x86_64 x86_64 x86_64 GNU/Linux

Subsystem

process

What steps will reproduce the bug?

TTY

$ node -e 'process.stdin; require("fs").readSync(0, Buffer.alloc(1))'

PIPE

$ cat | node -e 'process.stdin; require("fs").readSync(0, Buffer.alloc(1))'

How often does it reproduce? Is there a required condition?

Consistently reproducible by making use of the process.stdin getter.

What is the expected behavior?

Read a single byte from the standard input and exit gracefully.1

What do you see instead?

node:internal/fs/utils:345
    throw err;
    ^

Error: EAGAIN: resource temporarily unavailable, read
    at Object.readSync (node:fs:727:3)
    at [eval]:1:30
    at Script.runInThisContext (node:vm:129:12)
    at Object.runInThisContext (node:vm:305:38)
    at node:internal/process/execution:76:19
    at [eval]-wrapper:6:22
    at evalScript (node:internal/process/execution:75:60)
    at node:internal/main/eval_string:27:3 {
  errno: -35,
  syscall: 'read',
  code: 'EAGAIN'
}

Node.js v18.0.0

Additional information

The mere fact of accessing process.stdin causes libuv to set the O_NONBLOCK flag in the open file description, altering the behavior of fs.readSync(0, buffer) and throwing EAGAIN if there isn't enough data to be read.

Awareness

It looks like the behavior of that getter doing “things” is well–known internally, and several tests make use of this “feature”

// Just reference stdin, it should start it
process.stdin; // eslint-disable-line no-unused-expressions

// Reference the object to invoke the getter
process.stdin;

// This test *only* verifies that invoking the stdin getter does not
// cause node to hang indefinitely.
// If it does, then the test-runner will nuke it.
// invoke the getter.
process.stdin; // eslint-disable-line no-unused-expressions

Related

Footnotes

  1. Id est: the same behavior as running node -e 'require("fs").readSync(0, Buffer.alloc(1))' without calling the process.stdin getter first.

@0x2b3bfa0
Copy link
Author

0x2b3bfa0 commented Apr 22, 2022

Details

function defineStream(name, getter) {
ObjectDefineProperty(process, name, {
configurable: true,
enumerable: true,
get: getter
});
}

defineStream('stdin', getStdin);

switch (guessHandleType(fd)) {

CASE TTY

case 'TTY': {
const tty = require('tty');
stdin = new tty.ReadStream(fd);
break;
}

function ReadStream(fd, options) {

const tty = new TTY(fd, ctx);

const { TTY, isTTY } = internalBinding('tty_wrap');

node/src/tty_wrap.cc

Lines 140 to 143 in 1c4df35

TTYWrap::TTYWrap(Environment* env,
Local<Object> object,
int fd,
int* init_err)

*init_err = uv_tty_init(env->event_loop(), &handle_, fd, 0);

int uv_tty_init(uv_loop_t* loop, uv_tty_t* tty, int fd, int unused) {

uv__nonblock(fd, 1);

CASE PIPE

case 'PIPE':
case 'TCP': {
const net = require('net');

stdin = new net.Socket({
fd: fd,

node/lib/net.js

Line 291 in 7319419

function Socket(options) {

node/lib/net.js

Lines 356 to 358 in 7319419

this._handle = createHandle(fd, false);
err = this._handle.open(fd);

node/lib/net.js

Line 143 in 7319419

function createHandle(fd, is_server) {

node/lib/net.js

Lines 146 to 150 in 7319419

if (type === 'PIPE') {
return new Pipe(
is_server ? PipeConstants.SERVER : PipeConstants.SOCKET
);
}

node/lib/net.js

Lines 63 to 67 in 7319419

const {
Pipe,
PipeConnectWrap,
constants: PipeConstants
} = internalBinding('pipe_wrap');

node/src/pipe_wrap.cc

Lines 154 to 157 in 7319419

PipeWrap::PipeWrap(Environment* env,
Local<Object> object,
ProviderType provider,
bool ipc)

void PipeWrap::Open(const FunctionCallbackInfo<Value>& args) {

int err = uv_pipe_open(&wrap->handle_, fd);

int uv_pipe_open(uv_pipe_t* handle, uv_file fd) {

err = uv__nonblock(fd, 1);

@bnoordhuis
Copy link
Member

What you're describing is expected behavior; node has basically always behaved that way, at least since the libuv switchover in the v0.5 days, and probably before that.

That said, instead of putting the file descriptor into non-blocking mode, libuv could use sendmsg() and recvmsg() with the MSG_DONTWAIT flag - assuming all supported platforms support it.

@0x2b3bfa0
Copy link
Author

0x2b3bfa0 commented Apr 23, 2022

Updated #42826 (comment), happens also on pipes.

@bnoordhuis
Copy link
Member

Turns out sendmsg/recvmsg don't work on enough file descriptor types for my suggestion to be feasible. The current situation is probably as good as it gets.

@gireeshpunathil
Copy link
Member

thinking aloud: non-blocking behaviour of the standard streams and pipes make sense; but should the sync readers absorb EAGAIN and retry, instead of throwing? alternatively document the fact (that synchronous operations on non-blocking descriptors may throw based on the descriptor's current ability to respond to the request)?

@bnoordhuis
Copy link
Member

should the sync readers absorb EAGAIN and retry, instead of throwing?

No, because that effectively turns the operation into a busy loop. uv_fs_read() could block on poll(2) until the file descriptor becomes ready but that:

  1. feels like solving the problem in the wrong place

  2. doesn't necessarily work for all file descriptors types (can still get EAGAIN)

  3. interacts poorly with uv_tty_t I/O on the event loop thread (indeterminate which reader gets what) but the thing OP wants to do is prone to that anyway

uv_tty_t handles could do blocking I/O on a separate thread. Libuv already does that on macOS because tty file descriptors don't work with kqueue(2) (facepalm, Apple) but it's a heavy weight solution and adds more failure modes like "can't spawn thread".

document the fact that synchronous operations on non-blocking descriptors may throw

Sure, but that's like documenting water is wet.

@gireeshpunathil
Copy link
Member

thanks @bnoordhuis

No, because that effectively turns the operation into a busy loop.

https://nodejs.org/api/fs.html#synchronous-example already warns about implications of using sync variant as blocking the event loop for a period of time till the operation complete. But of course, agree that busy loop is not a good idea.

Sure, but that's like documenting water is wet.

This one I am finding it difficult to relate. At the system programming level, an I/O call on a non-blocking , non-ready descriptor throwing may be well understood, but at the Node.js API level where things are abstracted, this behaviour cannot be implied, and is not specified anywhere in the docs?

@bnoordhuis
Copy link
Member

OP's example is fs.readSync(0, buf). When you work at that level of abstraction, you're probably well aware of the difference between blocking and non-blocking I/O.

The surprise seems to be that stdio fds in node are sometimes blocking, sometimes non-blocking. I agree that's something worth documenting.

@bnoordhuis
Copy link
Member

I'm going to close this out. If anyone still wants to send a documentation pull request, please do.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants