-
Notifications
You must be signed in to change notification settings - Fork 30k
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
CTRL-C does not quit repl when in infinte loop #6612
Comments
I'm not sure this is actually possible to fix due to how the repl works. |
I’d like to try and take this one if nobody minds. |
I'm pretty sure that there is no way to get back to the repl after an infinite loop. Just CTRL+C twice will quite back to the terminal. |
As well as this there almost certainly isn't a fix |
@addaleax You're probably going to have to take a similar approach to what I did in #5904: start a watchdog thread, wake it from the SIGINT signal handler, and (presumably) make it call I suspect you'll need to insert a C++ stack frame between the REPL and the evaluated code, with a If you figure out how to fix the semaphore issue on FreeBSD, let me know. :-) EDIT: That's for Unices. I don't know how you'd approach this on Windows. |
How about just making Node.js REPL to be a pure proxy/manager to another node |
@bnoordhuis Yep, that sounds about right. I’d add an option to the I don’t have any local access to FreeBSD machines, though… are the CI runs from that PR still available somewhere? And would anything speak against |
@yorkie You mean, having a “front” REPL interface that proxies to another process which does what the current REPL does? That may be possible, but it would probably be a lot more intrusive. Any state would be lost in the case of killing it, so it’s not that big an improvement over the current situation… or am I misunderstanding something? |
@addaleax thanks for your comment, here is my respond :-)
Yea, exactly.
Could you please expand what do you mean by
Yea, when we externally kill that child process, the state would be get lost. But what's the real case? |
Probably not. I think they're kept around for 30 days. After that, they're deleted.
Instead of a semaphore? uv_async_send() doesn't claim to be async signal-safe; it should work in practice but that's more of an implementation detail than a guarantee. |
Sure, sorry for being unclear. I just think it would probably be more work to get the default eval of the REPL to use a child process for communication than it would be to catch a signal – I may be wrong about that, of course.
Not sure what you mean – the ideal way to resolve Ctrl+C would be to stop execution of the current command and return to the REPL, and still have everything from before it in place (unless, of course, the interrupted command modified something). |
Yes… but if I understood you correctly, |
You are right, just an advice :-) |
I looked into it just now. uv_sem_post() in #5904 seems to be working properly, it fixes the failing test. Another test, parallel/test-debug-signal-cluster, is now failing consistently (but only on freebsd) but that test has been extraordinarily flaky in the past; I'm rather distrustful of it. |
Adds the ability to stop execution of the current REPL command when receiving SIGINT. This applies only to the default eval function. Fixes: nodejs#6612 PR-URL: nodejs#6635 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
wow, thank you! |
Platform in Linux x64
Node version 6.0.0
repro:
CTRL-C does nothing
Since the process handles SIGINT fine when sent via kill I think this is due to
->Run
in v8 somehow blocking all keyboard input.The text was updated successfully, but these errors were encountered: