On OS X with GTK backend I get the following warning on destruction:
(geany:70109): GLib-GObject-CRITICAL **: g_object_ref: assertion 'object->ref_count > 0' failed
twice for every closed editor in Geany. I have bisected this issue to commit
6c3f6c0ec9e3a41bfc1f855523e2ccfbadf6ed84
When run in debugger with --g-fatal-warnings, I get the following backtrace:
g_logv + 433
frame #1: 0x0000000100bda8d6 libglib-2.0.0.dylibg_log + 134g_object_ref + 204
frame #3: 0x0000000100b4b901 libgobject-2.0.0.dylibg_value_object_collect_value + 49g_signal_emit_valist + 2057
frame #5: 0x0000000100b5eeb6 libgobject-2.0.0.dylibg_signal_emit + 134gtk_widget_unparent + 677
frame #7: 0x000000010008750f libgeany.0.dylibScintillaGTK::Destroy(_GObject*) + 31g_object_unref + 440
frame #9: 0x00000001002e6b96 libgtk-quartz-2.0.0.dylibgtk_box_forall + 54gtk_container_destroy + 76
frame #11: 0x0000000100b47432 libgobject-2.0.0.dylibg_closure_invoke + 290signal_emit_unlocked_R + 2993
frame #13: 0x0000000100b5e834 libgobject-2.0.0.dylibg_signal_emit_valist + 2164g_signal_emit + 134
frame #15: 0x00000001003c82d8 libgtk-quartz-2.0.0.dylibgtk_object_dispose + 40g_object_unref + 210
frame #17: 0x0000000100b4abbf libgobject-2.0.0.dylibg_cclosure_marshal_VOID__OBJECTv + 175_g_closure_invoke_va + 282
frame #19: 0x0000000100b5e57b libgobject-2.0.0.dylibg_signal_emit_valist + 1467g_signal_emit + 134
frame #21: 0x000000010001c815 libgeany.0.dylibremove_page(page_num=0) + 229 at document.c:737What I believe is happening here is that in #8 ScintillaGTK gets unreffed so at the point the ::Destroy() function is called it has 0 references.
In my opinion the gtk_widget_unparent() should be called in what corresponds to _dispose() in GTK. I'm not really familiar with C++ bindings but the ::Destroy() method seems to correspond to GTK's finalize().
(The OSX Gtk backend is sometimes a bit strange so it may behave a bit differently than on Linux.)
GTK+ on OS X is outside my experience. Your explanation of "dispose" seems to be in accord with the API documentation. Do you want to produce a patch for this?
There's nothing OS X specific really. See the attached patch. I tried to follow the style of Destroy() and added the try block even though nothing should throw an exception (but might be useful if something throwing an exception is added in the future). The dispose() seems to be called alright and fixes the issue for me.
Committed as [1acb50].
Related
Commit: [1acb50]
I am now seeing crashes in gtk_widget_unparent / ScintillaGTK::MainForAll / ScintillaGTK::ForAll when closing the about box in SciTE which contains a Scintilla instance.
Just tried but I can't reproduce it here. Could you post a full backtrace?
This is on Lubuntu 15.10.
OK, tried on normal Ubuntu 15.10 but couldn't reproduce it.
However, I checked what GtkScrolledWindow does and it also keeps a reference to the scroll bars (also uses destroy() instead of dispose() but this shouldn't matter IMO). Holding a reference to the scrollbar guarantees that the scrollbar object isn't destroyed in the middle of unparent() and that something else doesn't use the invalid object. Would you try if the attached patch helps?
The GObject docs state that
So AFAICT (and AFAIK),
dispose()is supposed to do what you initially did, but you also need to makeForAll()proof against scrollbars beingNULL(and potentially all other access, but I highly doubt anything else where it's used can be called after dispose() anyway).This said, in practice AFAIK this requirement of dropping references is to avoid cyclic references or something, by having all objects releasing each other's reference and only after that actually destroying the object; so it probably doesn't really matter here.
Ah, right, I misread the stack trace - I thought the crash happened during the invocation of unparent() in which case the scroll bars don't have the NULL value yet so the check wouldn't solve anything. But actually it happens in the super chain-up call so the crash must be caused by the NULL value - I'll send a new patch.
Here's the NULL check patch.
Comitted [876661].
Related
Commit: [876661]
@nyamatongwe do you confirm this fixed the crash you've been seeing? I'd like to backport this to Geany's 3.6.1 (https://github.com/geany/geany/pull/746), but as I wasn't able to reproduce any of this (neither @techee's warnings, nor yours with his patch, and not even in a Lubuntu VM), I'd rather be sure not to import something known to be buggy :)
Yes, the patch fixes the crash.
From Mitchell Foral:
My application now throws the following assertion errors upon calling
gtk_widget_destroy()on Scintilla widgets:Gtk-CRITICAL **: IAgtk_widget_unmap: assertion** `GTK_IS_WIDGET (widget)' failed
Gtk-CRITICAL **: IA__gtk_widget_unrealize: assertion `GTK_IS_WIDGET (widget)' failed
Gtk-CRITICAL **: IA__gtk_widget_unrealize: assertion `GTK_IS_WIDGET (widget)' failed
I've tracked these errors to `ScintillaGTK::UnRealizeThis()` and marked them with an '*' in the code snippet below.
The referenced changeset sets `scrollbarv` and `scrollbarh` to `NULL`, so the widget assertion error makes sense. However, the unmap call is weird because a `GTK_IS_WIDGET(widget)` test succeeds and unmap() is always called.
When I comment out the
introduced in the referenced changeset, my assertion errors go away.
I can't imagine how unrealize() could be called by Gtk after dispose (and if it was called before, the scrollbar widgets wouldn't be NULL). What I rather suspect here is that Mitchell over-destroys the Scintilla widget. From the Textadept screenshots it seems it uses a GtkNotebook for tabs. Now if he places the Scintilla widget inside the notebook's tab, all he needs to do to destroy the Scintilla widget is to call gtk_notebook_remove_page() - it should unref its children including the Scintilla widget that shuld get properly destroyed by this. Any explicit gtk_widget_destroy() on the Scintilla widget is extra IMO.
Mitchell could put some trace inside void ScintillaGTK::Destroy() to see if it gets called even without the explicit gtk_widget_destroy() call or to check if it gets called twice with the gtk_widget_destroy() call for a single file.
Forgot to mention that if the above isn't right, it would help to get a backgrace from gdb when running the editor with --g-fatal-warnings to know from where unrealize() gets called.