print JS stack unexpected signal like ctrl+c

[ry]

Imagine you have the program

while (true) {
  console.log("hello");
}

If one ctrl+c the process, it would be nice if we printed the JS stack.

[bartlomieju]

For reference, tokio has tokio-signal which can be used to intercept signals

[bartlomieju]

@mhvsa are you working on this issue?

[Cre3per]

My rough idea was to install a signal handler in Rust that sets a bool flag. In a separate thread, wait for the flag to be set, then call isolate.request_interrupt(), current_stack_trace(), isolate.terminate_execution(), without any changes to the TS-side. The thread+bool might be unnecessary with tokio-signal, I haven't looked into that yet.

@mhvsa implemented a ctrl+c handler, but it was rejected by @kevinkassimo because of non-generality. Kevin also talked about resolving TS-side promises.
Kevin wrote these comments before the addition of Deno.signal() in #3757. I'm wondering if those comments are still relevant?

On another note, this is a feature I as a user would dislike because I ctrl+c all the time and want to see me logs, rather than a stack trace. Should this feature be enabled by a command-line flag?

[bartlomieju]

Just FYI @Cre3per last time I tried it current_stack_trace() did not provide anything useful. At the same time request_interrupt() return an error that has no stack which also doesn't provide any useful info. Before trying to tackle it with signal handling I suggest to create a minimal example that would show that we are actually able to retrieve a stack trace at any point in time.

I'm also not particularly interested in implementing this feature. It's quite a lot of work for little gain.

[Cre3per]

@bartlomieju Thanks for reporting the outcome of your attempts.
I've got a minimal example working locally. It's incomplete, but enough for me to say that printing a stack trace on signal is possible.
"Incomplete" and "locally", because I had to patch rusty_v8. I'm not showing code yet, because this still is highly experimental.

I'm running a thread in JsRuntime::mod_evaluate() which calls

Isolate::request_interrupt() -> StackTrace::current_stack_trace() -> Isolate::terminate_execution()

This part is working, I see stack frames with line+column numbers.
The issue I'm facing is that we have no scope in the request_interrupt() handler, we only have an isolate. rusty_v8's StackTrace and StackFrame API need a scope parameter. They don't actually use the scope, they only need the isolate, that's my patch in rusty_v8.
Now I'm struggeling at getting the script name of a stack frame. There's an API, but I need to call Value::to_string(), which needs a scope. This time there is no easy patch I could make in rusty_v8, and I get a feeling that I should grab a scope from somewhere instead of touching rusty_v8.

I can't use the local scope or tc_scope in mod_evaluate(), because I also need the isolate, which leads to a double mutable borrow of the isolate. Also, I can't access those scopes in another thread. So I'm stuck with a function in another thread that has an isolate, but needs a scope.

Another idea that crossed my mind was to somehow freeze the isolate in the request_interrupt() handler and return execution to the main thread (Aborting Module::evaluate()). That way we wouldn't have to worry about v8 thread safety, as we could get the stack trace in the main thread. I haven't looked into that approach.

UPDATE: Callbacks can obtain a scope via CallbackScope. However, the stacktrace API needs a HandleScope<Context>. We can only construct HandleScope<()> with an isolate.
I don't think Isolate::GetCurrentContext() (not currently rusty) can be used. To handle lifetimes, rusty_v8 would need the scope used to create the context. We don't have access to that scope, and it's not Sendable.

UPDATE: Tried getting the global_context() into the request_interrupt() callback using unsafe-send-sync (Because the context isn't accessed in the thread). CallbackScope cannot be constructed from Global<Context>.

[bartlomieju]

@Cre3per nice find! Please open a PR to rusty_v8, I will discuss this with @piscisaureus who might be able to advise how to tackle this

[Cre3per]

I'm giving up on this issue for the time being.
I have a semi-working solution that involves adding a trampoline to the request_interrupt() callback in rusty_v8 to add a CallbackScope using Isolate::GetCurrentContext().

My changes to rusty_v8: https://github.com/denoland/rusty_v8/commit/c8114005a4d42db4081874e25fe4ecbbe687de1a My changes to deno: https://github.com/denoland/deno/commit/12cee9dd99aab743cfb900cb71ac071d4630c62f

The major issues of my implementation in rusty_v8, for anyone interested in continuing:

• The data argument of request_interrupt() can no longer be set by the user, because I'm using it to transport the original callback. request_interrupt()'s callback is not called immediately, so we'd have to transport the user's data pointer on the heap.
• There's a good chance calling Isolate::GetCurrentContext() and Local::from_raw() is wrong or unsafe. I lack the experience in v8 to judge.

The real issue I see in this feature request is the general lack of scopes in rusty_v8 callbacks. Relevant comment https://github.com/denoland/rusty_v8/pull/406#issuecomment-646993935