You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
CTRL-C does not quit repl when in infinte loop #6612
@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 isolate->TerminateExecution().
I suspect you'll need to insert a C++ stack frame between the REPL and the evaluated code, with a TryCatch try_catch that calls isolate->CancelTerminateExecution() when try_catch.HasTerminated() is true.
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.
I’d add an option to the vm.runIn(This)Context functions for catching the signal… that doesn’t seem too hard, there’s already a lot in place to support the timeout argument and a lot of the timeout Watchdog class seems reusable.
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 uv_async_send for waking up the thread? That uses only write() under the hood, as far as I can tell.
@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?
are the CI runs from that PR still available somewhere?
Probably not. I think they're kept around for 30 days. After that, they're deleted.
And would anything speak against uv_async_send for waking up the thread?
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.
Could you please expand what do you mean by intrusive?
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.
But what's the real case?
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).
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.
Yes… but if I understood you correctly, uv_sem_post suffers from the reverse problem?
if I understood you correctly, uv_sem_post suffers from the reverse problem?
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.
Activity
Fishrock123 commentedon May 6, 2016
I'm not sure this is actually possible to fix due to how the repl works.
addaleax commentedon May 6, 2016
I’d like to try and take this one if nobody minds.
bnoordhuis commentedon May 6, 2016
@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
isolate->TerminateExecution().I suspect you'll need to insert a C++ stack frame between the REPL and the evaluated code, with a
TryCatch try_catchthat callsisolate->CancelTerminateExecution()whentry_catch.HasTerminated()is true.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.
addaleax commentedon May 6, 2016
@bnoordhuis Yep, that sounds about right.
I’d add an option to the
vm.runIn(This)Contextfunctions for catching the signal… that doesn’t seem too hard, there’s already a lot in place to support thetimeoutargument and a lot of the timeoutWatchdogclass seems reusable.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
uv_async_sendfor waking up the thread? That uses onlywrite()under the hood, as far as I can tell.addaleax commentedon May 6, 2016
@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?
bnoordhuis commentedon May 6, 2016
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.
addaleax commentedon May 6, 2016
@yorkie
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).
addaleax commentedon May 6, 2016
@bnoordhuis
Yes… but if I understood you correctly,
uv_sem_postsuffers from the reverse problem?bnoordhuis commentedon May 6, 2016
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.
6 remaining items
SamSaffron commentedon Jun 19, 2016
wow, thank you!
repl: break on sigint/ctrl+c