Signals: re-throw exceptions in handlers

[romgrk]

Pass tests/object__signal__throws.js

[romgrk]

Problem: for synchronous events (triggered by a js function), errors are thrown correctly, but for asynchronous events (eg callback when a user presses a key) the errors are not thrown the same way. The error is then noticed in some other part of the code, when we try to unwrap a Maybe.

Synchronous:

const entry = new Gtk.Entry()
entry.on('editing-done', () => { throw new Error('test') })
entry.editingDone()

Asynchronous:

const entry = new Gtk.Entry()
entry.on('key-press-event', () => { throw new Error('test') })
// user presses a key

[benwaffle]

• Is there a mechanism in C for erroring in a signal handler?
• What does pygobject do?
• Rust?
• In C++ it seems that uncaught errors crash the program - https://developer.gnome.org/gtkmm-tutorial/stable/sec-exceptions-in-signal-handlers.html

[romgrk]

Ok, got more infos: the problem is that when an error happens inside a signal handler callback and gtk_main is running, NodeJS/V8 can't do it's thing: throw the error and emit the process 'uncaughtException' event. (because gtk_main is an event loop that doesn't return until it's told to)
So in src/closure.cc @ Closure::Marhsal, if we call gtk_main_quit when an error is thrown, the problem is solved. However, it adds a dependency on gtk+-3.0 being there at compile time, and I don't like that solution very much.

PyGObject seems to wrap gtk_main in overrides, with stuff that seems to deal with signals, and seems to call gtk_main_quit when SIGINT is called.

@override(Gtk.main)
def main(*args, **kwargs):
    with register_sigint_fallback(Gtk.main_quit):
        with wakeup_on_signal():
            return _Gtk_main(*args, **kwargs)

PyGObject doesn't have a dependency on gtk at compile time.

So I'm not sure if that's what they're doing, but we could try to achieve something similar, and emit a SIGINT in Closure::Marshal (our signal C++ callback) if an error is thrown from the JS side.

[benwaffle]

isn't the libuv event loop nested inside the GTK event loop? This should allow 'uncaughtException' to be thrown.

[romgrk]

Yes, it is nested inside the Gtk event loop. I don't know if it should allow it, but it's not the current behavior. The 'uncaughtException' isn't thrown until gtk_main exits.

I'll maybe check a few more bindings implementations, and I'll implement it as PyGObject above if I don't find any other magical trick.

(Ping @magcius, following your comment, if you have any hints that could help here it would be awesome!)

[magcius]

What in V8 emits the uncaughtException handler? Did you guys integrate the more robust threading solution made for eos-knowledge-node-content for mainloop nesting?

[romgrk]

No, it's not integrated and I don't know what it is. Do you have any links to eos-knowledge-node-content?

Meanwhile, it seems that gtk_main also prevents other process.on events:

/*
 * Gtk-3.0.js
 */

exports.apply = (Gtk) => {

    const originalMain = Gtk.main
    Gtk.main = function() {
        process.on('SIGINT', () => {
            console.log('SIGINT raised') // Never called
            Gtk.mainQuit()
        })
        originalMain()
    }
}

[benwaffle]

I assume https://github.com/endlessm/eos-knowledge-content-node

[magcius]

Yes, that's it. See this comment in node-gir: Place1/node-gir#24 (comment)

Otherwise, I do pump the libuv event loop, so if uncaughtException happens because of that, it should be fine. Looking at what happened, it seems that someone ported the closure.cc code to NaN (as I pleaded when people started developing this, please don't use NaN, it doesn't work), and NaN automatically captures exceptions and automatically ticks the mainloop, according to this comment: nodejs/nan#284 (comment)

[magcius]

There's apparently some replacement for NaN called N-API now, but I've never used it.

[romgrk]

I've tried removing all NaN stuff from closure.cc but the issue persist, so I've restored it. Some NaN is essential because of incompatibilities between versions (e.g. func->Call in node 8.x doesn't have the same signature as node 10.x). We're really just using Nan::Call here, no Nan::MakeCallback.

I've read a bit about N-API and it seemed interesting, but not enough to warrant a re-write.

I've started the integration of mainloop nesting of eos-knowledge-node-content loop solution in #52, but after some testing, it doesn't seem to fix the issue here (though it might fix other issues). I'll keep investigating what's going on.

[magcius]

I still assume something is just capturing the exception somewhere.

[romgrk]

Got it, will investigate further.

process uncaughtException is emitted from:

• https://github.com/nodejs/node/blob/cf37945b121d23c044cd04ac79cd5ab312a34cbe/src/node.cc#L1407
• https://github.com/nodejs/node/blob/cf37945b121d23c044cd04ac79cd5ab312a34cbe/lib/internal/bootstrap/node.js#L472

Essentially, we'd just need to call process._fatalException(error) to trigger the uncaughtException, but I'll try to fix the underlying problem.

[romgrk]

So I went a different direction to solve this, I created a loopStack array shared between JS and C++ that contains the functions to quit the running loops. E.g. for gtk_main, gtk_main_quit is pushed onto the stack. This pushing is done in overrides. This is similar to what PyGObject does, by specifing in the overrides the function to call when an error occurs. Details in https://github.com/romgrk/node-gtk/pull/52/files.

@magcius, how did the solution for endless work for you? What problems did it solve, or on what aspects is it better than the previous loop.cc you wrote?

[magcius]

The goal of the Endless solution was to let both the Electron/Chromium and glib event loops run in parallel. If we think this is an issue where gtk_main() pauses the libuv loop and my hack to pump it isn't working, the thread solution could be something to try.