gtk-rs / gtk

DEPRECATED, use https://github.com/gtk-rs/gtk3-rs repository instead!
https://gtk-rs.org/
MIT License
1.25k stars 82 forks source link

are we sure destroy() is never needed? #1046

Closed emmanueltouzery closed 4 years ago

emmanueltouzery commented 4 years ago

Hi, I'm having an issue where dialog.destroy() works just fine, but dialog.close() doesn't.

I'm using relm and nested dialogs. First my dialog, which I display with run(), then triggered from that dialog, a gtk::FileChooserDialog, which I also trigger with run().

If after the run() for the FileChooserDialog I call destroy() on that dialog, everything works fine. No issues.

If however I call close() then the file chooser does not close, but seems to hang. My own dialog is OK and responsive, but the file chooser dialog does repaint, but is not responsive to clicks or even window close shortcuts.

I wonder if it's my mistake for having nested dialogs with run().. or if it's the decision to make destroy() unsafe in gtk-rs which is maybe not the right call for all the situations? I've tried turning my dialog in a gtk::Window and then the hanging problems go away, however then my 'dialog' is displayed as a window in i3 (not floating), even if I pass GDK_WINDOW_TYPE_HINT_DIALOG.

emmanueltouzery commented 4 years ago

I also tried std::mem::drop() on the file chooser dialog, but that doesn't help, the dialog stays displayed but hung.

EDIT: as an aside, I wonder (naively for sure) whether destroy could be made safe by changing its signature from &self to self -- making sure the object cannot be used afterwards.

sdroege commented 4 years ago

I'm using relm and nested dialogs.

That's a bad idea. Don't use recursive main loops (i.e. dialog.run()) but just show them and handle the response via the callback. Maybe that's the main problem?

or if it's the decision to make destroy() unsafe in gtk-rs which is maybe not the right call for all the situations?

You're free to call destroy() if you think it's safe in your situation (that's what unsafe blocks are for!), but in general it's unsafe to call :)

as an aside, I wonder (naively for sure) whether destroy could be made safe by changing its signature from &self to self -- making sure the object cannot be used afterwards.

That doesn't help because widgets/windows are refcounted. You could clone() it first, then destroy the clone and have another reference that you can still use.


Generally, can you provide a minimal testcase for reproducing this problem? It might be a bug in GTK, or it might simply be because you use recursive main loops which is discouraged.

emmanueltouzery commented 4 years ago

ok, i got it to work now. Sorry, I wanted some confirmation also because most of the examples in the gtk apidocs actually encourage ::run and ::destroy.. For instance https://developer.gnome.org/gtk3/stable/GtkMessageDialog.html#refsect1

Generally speaking I had several issues:

  1. the mistaken impression that I should either use a dialog with run() or a window with show_all (reality: it's OK to use dialog with show_all)
  2. it worked with destroy and the gtk apidocs mention destroy
  3. on top of that, the relm widget being dropped in my code, which made it stop triggering events when the "run" wouldn't block the execution, therefore making sure the relm widget wasn't dropped was pushing me towards the run() model (because it stopped working when i moved to show_all..)

Thank you for the hints, that helped!

GuillaumeGomez commented 4 years ago

Maybe we should add documentation about this behaviour in the Dialog documentation page? What do think @sdroege ?

sdroege commented 4 years ago

Which part exactly? But sure, I just never succeed to add things to the docs in the bindings :)

GuillaumeGomez commented 4 years ago

It's been greatly improved. :p

sdroege commented 4 years ago

But what do you want to document? run() should also be discouraged in the C docs and probably every GTK developer would tell you that nowadays

emmanueltouzery commented 4 years ago

yes, somehow it seems it's the upstream gtk docs that should be improved, because i don't see anywhere run() being discouraged in the upstream gtk apidocs

GuillaumeGomez commented 4 years ago

Hum, actually maybe upstream is better... I was sure it was already marked as deprecated there...