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

tty.ReadStream.isRaw is not reliable on child processes #47938

Closed
jrvidal opened this issue May 9, 2023 · 9 comments
Closed

tty.ReadStream.isRaw is not reliable on child processes #47938

jrvidal opened this issue May 9, 2023 · 9 comments

Comments

@jrvidal
Copy link
Contributor

jrvidal commented May 9, 2023

Version

v20.1.0

Platform

Linux 6.2.6-76060206-generic #202303130630168132977822.04~d824cd4 SMP PREEMPT_DYNAMIC Wed A x86_64 x86_64 x86_64 GNU/Linux

Subsystem

tty

What steps will reproduce the bug?

const child_process = require("child_process");

const main = process.argv[2] == null;

if (main) {
  process.stdin.setRawMode(true);

  console.log("parent:", process.stdin.isRaw);

  child_process.spawn("node", [__filename, "child"], { stdio: "inherit" });
} else {
  console.log("child:", process.stdin.isRaw);
}

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

Always

What is the expected behavior? Why is that the expected behavior?

I expect both logs to print true:

parent: true
child: true

What do you see instead?

The child process log prints false:

parent: true
child: false

Additional information

It seems that isRaw is unconditionally initialized to false (here), regardless of the actual state of the underlying tty.

I don't see anything obvious in libuv that would help initializing this flag, so maybe it's blocked on upstream?

@bnoordhuis
Copy link
Member

What you're seeing is the expected behavior and not a bug. Node and libuv go to great lengths to ensure stdio settings in process A don't affect process B. In the deep and dark past both would have printed 'true' but that was fixed aeons ago.

@jrvidal
Copy link
Contributor Author

jrvidal commented May 10, 2023

But those 2 processes read from the same TTY device, right? So the tty being in raw mode affects all processes equally.

EDIT: consider this variation, for instance:

const child_process = require("child_process");

const main = process.argv[2] == null;

if (main) {
  process.stdin.setRawMode(true);

  console.log("parent:", process.stdin.isRaw);

  child_process.spawn("node", [__filename, "child"], { stdio: "inherit" });

  process.stdin.once("data", (chunk) => console.log("parent got", chunk));
} else {
  setTimeout(() => {
    console.log("child:", process.stdin.isRaw);

    process.stdin.once("data", (chunk) => {
      console.log("child got", chunk);
      process.exit(0);
    });
  }, 2000);
}
  • node test.js
  • Press a (quickly)
  • Wait for child: false
  • Press a again

@bnoordhuis
Copy link
Member

Same tty, different file descriptors. Libuv reopens /dev/tty.

@jrvidal
Copy link
Contributor Author

jrvidal commented May 10, 2023

So if I go and run tcgetattr() for the second file descriptor (the one in the child), are you saying I'll find it to be in canonical mode? I suspect the answer is no. See my example above, for instance.

@bnoordhuis
Copy link
Member

Insofar they're not global settings like line speed, yes. It's not 100% infallible (e.g. doesn't work when stdio is a master pty) but most of the time libuv does the right thing.

Apropos the .isRaw property, that only reflects whether .setRawMode() was called. The documentation for .isRaw is somewhat ambiguous but the entry for .setRawMode()makes it clear. You're welcome to send a pull request to clarify the former if you want.

I'm going to close this but I filed libuv/libuv#3988 to discuss if libuv should explicitly put the tty in cooked mode on init. (It currently doesn't but node assumes it does.)

@bnoordhuis bnoordhuis closed this as not planned Won't fix, can't repro, duplicate, stale May 11, 2023
@jrvidal
Copy link
Contributor Author

jrvidal commented May 11, 2023

Documenting the quirk is fine, thanks.

Insofar they're not global settings like line speed, yes

Sorry, I'm genuinely puzzled by this answer. Are you saying "yes, tcgetattr() would find the terminal to be in canonical mode"? Because my question was semi-rhetorical: I know that it does not.

if libuv should explicitly put the tty in cooked mode on init

Regardless of what libuv does, I would find that behavior very surprising.

@bnoordhuis
Copy link
Member

I'm afraid I don't understand what you find puzzling about that comment. If you want an example in the opposite direction (things libuv can control without affecting other processes): the O_NONBLOCK flag.

Regardless of what libuv does, I would find that behavior very surprising.

How so?

@jrvidal
Copy link
Contributor Author

jrvidal commented May 11, 2023

I'm afraid I don't understand what you find puzzling about that comment

I'm just unsure whether we agree on the fact that, on the example above, if one were to run tcgetattr() on fd 0 on the child process, it would show that the terminal is in raw mode (disabled ICANON, etc.).

How so?

If I'm understanding correctly, reading from process.stdin for the first time would always reset to canonical mode, right? I don't know, it seems surprising that spawning a child ends up changing settings set by the parent. But I admit I can't think of a valid use case that would be trumped.

@bnoordhuis
Copy link
Member

Perhaps the confusion stems from you editing #47938 (comment) around the same time I replied.

We're in agreement assuming the operating system shares termios settings between the reopened /dev/pts/<fd> and the "top" pty. Don't quote me on it but I believe most modern Unices work that way.

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

2 participants