node-gtk icon indicating copy to clipboard operation
node-gtk copied to clipboard

Gtk.main() locks and prevents WebSocket callback from firing

Open Zipdox opened this issue 3 years ago • 13 comments

const WebSocket = require('ws');
const gi = require('node-gtk')
const Gtk = gi.require('Gtk', '3.0')

const socket = new WebSocket('wss://mppclone.com:8443/'); // socket from multiplayerpiano.com as example
socket.on('message', console.log);

gi.startLoop();
Gtk.init();

const win = new Gtk.Window();
win.on('destroy', () => Gtk.mainQuit());
win.on('delete-event', () => false);

win.setDefaultSize(200, 80);
win.add(new Gtk.Label({ label: 'Hello Gtk+' }));

win.showAll();
Gtk.main(); // comment out this line and it will spit out the websocket message

Zipdox avatar Apr 25 '21 00:04 Zipdox

Haven't had time to look into this yet, but I'll comment here for your information. This use-case is something that should be supported. Last time I played with the event loop I had the impression that async code was working, quick investigation seems to disprove that. The event loop integration is a bit sensible because we're stopping nodejs' UV loop to replace it with GLib's loop.

The compromise I had settled on would be that async code can be spawned alongside GLib's main loop, but all async handlers must be spawned from within GLib's loop, which means after Gtk.main() is called. To do so, one needs to attach a signal handler that will be called once the loop is started. This usually means something like gtkApplication.on('activate' () => { ...async code }) or gtkWindow.on('show', () => { ...async code }. However I've tried running your code like that and it didn't work, so I'll get back with more info once I know what's failing.

romgrk avatar Apr 26 '21 06:04 romgrk

hey fellas, wanna see something freaky? 🥴

run this

console.log('bo!');

const WebSocket = require('ws');
const gi = require('node-gtk')
const Gtk = gi.require('Gtk', '3.0')

const socket = new WebSocket('wss://mppclone.com:8443/'); // socket from multiplayerpiano.com as example
socket.on('message', console.log);

gi.startLoop();
Gtk.init();

const win = new Gtk.Window();
win.on('destroy', () => Gtk.mainQuit());
win.on('delete-event', () => false);

win.setDefaultSize(200, 80);
win.add(new Gtk.Label({ label: 'Hello Gtk+' }));

win.showAll();
Gtk.main(); // comment out this line and it will spit out the websocket message

its the same code but with console.log('bo!'), with it you get websocket messages in the console as you should.

and it is not just output buffering or some like that, it actually blocks something deeper, you can see it if you try to avoid console altogether and just update the ui with something like this

//console.log('bo!'); // uncomment this and the label will update on message

const WebSocket = require('ws');
const gi = require('node-gtk')
const Gtk = gi.require('Gtk', '3.0')

const socket = new WebSocket('wss://mppclone.com:8443/'); // socket from multiplayerpiano.com as example

gi.startLoop();
Gtk.init();

const win = new Gtk.Window();
win.on('destroy', () => Gtk.mainQuit());
win.on('delete-event', () => false);

win.setDefaultSize(200, 80);
const label = new Gtk.Label({ label: 'Hello Gtk+' });
win.add(label);
socket.on('message', message => label.setText(message));

win.showAll();
Gtk.main(); // comment out this line and it will spit out the websocket message

not sure why, but sure that this took me the better half of my morning to pinpoint this and also sure that my old java teach had a saying "if you add logging and it suddenly works - youve done f'ed up"

also, @Zipdox i saw in #288 that you work around this with worker threads, you can just pop Gtk.main() into a process.nextTick(() => Gtk.main()) or anything like that and it does not block none (as long as you console.loged something apparently). That is how i got into this issue, i had my app originally working fine with process.nextTick (and console.log apparently) but wanted to put gtk in a worker thread, got your #288 error and started digging.

@romgrk sorry i dont have any more details on "why tho" for this, i dont really have the time currently to dig around inside native modules, which is sad cause this is a very freaky and cool bug)

ValentineStone avatar May 14 '21 07:05 ValentineStone

Yes that's something I had already noticed some time ago. I've done some work in #265 to try to fix the issue, but I haven't had time to test all the use cases and TBH the problem is maybe technically impossible to solve in node-gtk.

The issue with the event loop is that Gtk.main() (or any GLib's MainLoop using function) will basically interrupt nodejs' event loop mid-cycle, and replace it with GLib's event loop, and manually call uv_run() at each cycle of GLib's loop. I'm not 100% sure what's going on here, but my guess is the async events that are being handled before nodejs' loop is interrupted are forgotten until the loop is called again, because GLib doesn't know those events need to be processed. I've tried circumventing the issue by calling process._tickCallback() right after the loop is started, but so far it doesn't seem to fix the problem completely. One workaround that might work would be to spawn all nodejs async tasks from within GLib's loop, eg you call Gtk.main(), then you create your websocket on some event callback (GtkWindow's show or GtkApplication's activate). Can't guarantee it will work though.

So the question basically is how to properly intertwine both event loops. I've started investigating the question, and there are 2 approaches:

  1. Implement the required primitives in GLib to allow pluggable event loops: https://gitlab.gnome.org/GNOME/glib/-/issues/693
  2. Use the same approach as node-qt and run both loops in separate threads: https://github.com/nodegui/qode

I'm still unsure which approach is the best, I'll probably need to implement something to try to understand more.

romgrk avatar May 14 '21 13:05 romgrk

I specifically use this to avoid the likes of qt's qode and yue's yode, cause i run it on weird iot operating systems, and recompiling custom node versions for them is cumbersome. But other ui libs do this for a reason, so i might just have to deal with it eventually)

thanks for the GLib link, ill check it out)

ValentineStone avatar May 14 '21 13:05 ValentineStone

I think we could get away with just node but separate threads. IIUC, the problem is that Qt absolutely wants to be in the main thread, therefore it cannot be spawned from the nodejs main thread. AFAICT, Gtk doesn't have that requirement.

romgrk avatar May 14 '21 14:05 romgrk

I specifically use this to avoid the likes of qt's qode and yue's yode, cause i run it on weird iot operating systems, and recompiling custom node versions for them is cumbersome. But other ui libs do this for a reason, so i might just have to deal with it eventually)

thanks for the GLib link, ill check it out)

For nearly the same reason, I will also favour trying to keep it working with upstream Nodejs at any cost. That's one very big differentiating advantage vs node-qt.

If GTK can indeed be ran in a fully compartmentalised thread, allowing to completely forget about event loop integration on the C side as such, it will also be quite a boon I believe.

We have no idea where future I/O work will take nodejs. It may well go further, and harder on things like Uring, for nodejs being a server platform.

baybal avatar Jun 07 '21 19:06 baybal

I also prefer to avoid a forked nodejs. Don't have anything to add regarding solution to this issue at the moment, any solution needs research before commenting more.

romgrk avatar Jun 07 '21 23:06 romgrk

I recently ran into a similar issue. I needed to show a notification using Notify.Notification with a custom action that opens a window.

Worker threads were kind of helpful. I've encapsulated everything related to Gtk into a separate gtk-main-worker.js file and configured a worker on the parent side:

const gtkMainWorker = new Worker(path.join(__dirname, 'gtk-main-worker.js'));
function showNotification(title: string, message: string, icon?: string) {
  gtkMainWorker.postMessage({ title, message, icon });
}

And gtk-main-worker.js itself:

import { parentPort } from 'worker_threads';
import gi from 'node-gtk';
const gtk = gi.require('Gtk', '3.0');
const notify = gi.require('Notify');

// No gi.startLoop() here
gtk.init();
notify.init('Test App');

let notification;

function showNotification(title: string, message: string, icon: string) {
  notification = new notify.Notification({
    summary: title,
    // ...
  });
  notification.addAction('default', 'open', () => showDetails(title, message));
  notification.connect('closed', () => gtk.mainQuit());
  notification.show();
  gtk.main();
}

function showDetails(title: string, message: string) {
  const win = new gtk.Window({ title });
  win.connect('destroy', gtk.mainQuit);

  win.add(new gtk.Label({ label: message }));

  win.showAll();
  gtk.main();
}

parentPort.on('message', ({ title, message, icon }) => {
  showNotification(title, message, icon);
});

WebSocket listener on the parent side works fine but as long as the window is open the worker is blocked and can't receive messages from the parent. In other words, if the window is open new notifications won't be showing but WebSocket messages still can be processed.

black-roland avatar Nov 14 '21 21:11 black-roland

I have recently discovered the wonders of GLib and libsoup etc. in C and have found myself thinking that maybe JavaScript isn't the appropriate language to use with GTK. Perhaps using Vala is a better solution after all.

Zipdox avatar Nov 15 '21 00:11 Zipdox

WebSocket listener on the parent side works fine but as long as the window is open the worker is blocked and can't receive messages from the parent. In other words, if the window is open new notifications won't be showing but WebSocket messages still can be processed.

Consider putting GTK in the parent and doing the rest in the worker.

Zipdox avatar Nov 15 '21 00:11 Zipdox

I am looking whether there is any chance to splice libuv functionality exposed by node api anywhere in the event loop https://docs.gtk.org/glib/main-loop.html

baybal avatar Nov 15 '21 01:11 baybal

My apologies in advance for asking questions instead of providing helpful input

Use the same approach as node-qt and run both loops in separate threads

Any known downside to this approach? I was looking into this library as a source of inspiration for a possible Deno port but can't quite wrap my head around why many bindings go down the path of mixing the two loops or using only one (e.g. GJS uses only GLib's loop)

z0al avatar May 19 '22 19:05 z0al

Any known downside to this approach? I was looking into this library as a source of inspiration for a possible Deno port but can't quite wrap my head around why many bindings go down the path of mixing the two loops or using only one (e.g. GJS uses only GLib's loop)

Race conditions. Although GTK methods are mutexed, I don't think V8 calls are. So you would be able tot call GTK methods just fine, but if you set a callback function for a GTK event, it could cause problems. I'm not actually sure about that though.

Zipdox avatar May 20 '22 12:05 Zipdox