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

Node.js REPL freezes with "while (true);" or "while (true){}" input #54193

Closed
dmand-commits opened this issue Aug 3, 2024 · 14 comments
Closed
Labels
confirmed-bug Issues with confirmed bugs. inspector Issues and PRs related to the V8 inspector protocol repl Issues and PRs related to the REPL subsystem. v8 engine Issues and PRs related to the V8 dependency.

Comments

@dmand-commits
Copy link

Version

v22.5.1

Platform

Linux linux 6.10.2-arch1-1 #1 SMP PREEMPT_DYNAMIC Sat, 27 Jul 2024 16:49:55 +0000 x86_64 GNU/Linux

Subsystem

no modules were involved

What steps will reproduce the bug?

Open Node.js interactive mode by typing node in the terminal.
2. Enter while (false){} or while (false);.

  • Notice the "undefined" display ready to be output.
  1. Enter while (true); or while (true){}.
    • The terminal freezes and becomes unresponsive.

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

It reproduces 100% of the time, the required condition is that you're on node.js interactive shell.

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

the expected behavior would be the return value of the block of code used in the while-loop, since 'while (false);' returned undefined, something predictably consistent is also expected with 'while (true);'

What do you see instead?

The terminal stops working until I kill the process manually with htop or kill command from another terminal.

Additional information

the issue is that in node.js interactive mode, when I type "while (false){}" or "while (false);" it will make a vague display of undefined ready to be output, sort of like a google suggestion. Now, here's the real deal issue: try typing "while (true);" or "while (true){}" instead. Your terminal will freeze and the only way to stop node.js at that point would be to kill the process with kill or htop. I think I know the reason behind the issue: the "semi-google-suggestion-like" vague display. Its purpose is it show you what is ready to be displayed when you press Enter, to save you ease and time by assuming that you likely are just wanting to glance at outcomes oftentimes. But to achieve this process of simulating the google-like output display, it has to execute the command in the background, and wait for the return result of executing that command, then send it to the vague display, therefore resulting in the bug that freezes your terminal when you type "while (true);" in interactive mode.

@jakecastelli jakecastelli added the repl Issues and PRs related to the REPL subsystem. label Aug 4, 2024
@avivkeller
Copy link
Member

avivkeller commented Aug 4, 2024

This seems to be working as intended.

While loops block the main thread, meaning no other execution can occur, hence the hanging.

I've optimisticly marked this as wontfix, but another member of the org can always change that. If members agree, please close the issue as "Not planned".

@avivkeller avivkeller added the wontfix Issues that will not be fixed. label Aug 4, 2024
@jfhr
Copy link

jfhr commented Aug 4, 2024

I believe the issue is that the repl hangs when you type in while(true); without pressing enter (meaning the preview evaluation causes the hanging). It doesn't happen when running with TERM=dumb (which disables the preview) and also doesn't seem to happen on node 20.

The preview evaluation works by sending a Runtime.evaluate command to the v8 inspector with a timeout of 333ms, but it looks like the timeout is ignored in this case.

@avivkeller avivkeller removed the wontfix Issues that will not be fixed. label Aug 4, 2024
@avivkeller
Copy link
Member

Oh okay. I've adjusted the labels, thanks!

@dmand-commits
Copy link
Author

dmand-commits commented Aug 5, 2024 via email

@BridgeAR
Copy link
Member

BridgeAR commented Aug 5, 2024

The preview should be limited to 333ms. That's the hardcoded limit that is defined in the code. Could someone check if this is an issue with Node.js vs V8?

@MrJithil
Copy link
Member

MrJithil commented Aug 9, 2024

Looking in to it @BridgeAR

@MrJithil
Copy link
Member

Its an issue from v8. Reporting to v8 and will analyze there.

@MrJithil
Copy link
Member

Please close the ticket as known deps issue.

@avivkeller
Copy link
Member

avivkeller commented Aug 12, 2024

@MrJithil could you provide more information about why this is a known issue, what exactly goes wrong on V8?

Thanks!

@avivkeller
Copy link
Member

I see. Runtime.evaluate is ignoring the timeout.

@avivkeller avivkeller added confirmed-bug Issues with confirmed bugs. v8 engine Issues and PRs related to the V8 dependency. inspector Issues and PRs related to the V8 inspector protocol and removed repro-exists labels Sep 19, 2024
@avivkeller
Copy link
Member

@nodejs/v8 the runtime inspector is not respecting the timeout parameter

@MrJithil
Copy link
Member

MrJithil commented Oct 6, 2024

Yes. This has to be fixed in v8. Thats why it's a known dep issue.

@MrJithil
Copy link
Member

MrJithil commented Oct 6, 2024

@legendecas
Copy link
Member

legendecas commented Oct 17, 2024

This was introduced in #49639. V8 option --no-use-osr can be a workaround to fix the issue.

Running with --trace-osr, the log shows that there is an infinite loop around osr compilation:

[OSR - compilation started. function: , osr offset: 2, mode: ConcurrencyMode::kConcurrent]
[OSR - unavailable (failed or in progress). function: , osr offset: 2, mode: ConcurrencyMode::kConcurrent]
[OSR - compilation started. function: , osr offset: 2, mode: ConcurrencyMode::kConcurrent]
[OSR - unavailable (failed or in progress). function: , osr offset: 2, mode: ConcurrencyMode::kConcurrent]
[OSR - compilation started. function: , osr offset: 2, mode: ConcurrencyMode::kConcurrent]

V8 Issue: https://issues.chromium.org/issues/374013413

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed-bug Issues with confirmed bugs. inspector Issues and PRs related to the V8 inspector protocol repl Issues and PRs related to the REPL subsystem. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants