search

nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
106.56k stars 29.05k forks source link

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

Open dmand-commits opened 1 month ago

dmand-commits commented 1 month ago

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.

  1. Enter while (false){} or while (false);.
    • Notice the "undefined" display ready to be output.
  2. 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.

RedYetiDev commented 1 month ago

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".

jfhr commented 1 month ago

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.

RedYetiDev commented 1 month ago

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

dmand-commits commented 1 month ago

Yeah, exactly, the issue is with the preview, which simulates code execution and relies on it completing to show you the preview; if the code never finishes execution theoretically the response keeps waiting indefinitely and the tty is rendered unresponsive.

On Sun, Aug 4, 2024 at 6:30 PM Aviv Keller @.***> wrote:

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

— Reply to this email directly, view it on GitHub https://github.com/nodejs/node/issues/54193#issuecomment-2267581307, or unsubscribe https://github.com/notifications/unsubscribe-auth/BJ6CR56VXKRXVU6JYGUQ7HDZPZCI7AVCNFSM6AAAAABL6CU246VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDENRXGU4DCMZQG4 . You are receiving this because you authored the thread.Message ID: @.***>

BridgeAR commented 1 month ago

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 commented 1 month ago

Looking in to it @BridgeAR

MrJithil commented 1 month ago

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

MrJithil commented 1 month ago

Please close the ticket as known deps issue.

RedYetiDev commented 1 month ago

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

Thanks!

RedYetiDev commented 22 hours ago

I see. Runtime.evaluate is ignoring the timeout.