Bug 275366

In fact, in this case liboxygen-gtk.so gets unloaded before most (or all) of destructors are called. dlclose is called by gtk_theme_engine_unload(), which in turn calls theme->exit(), which points to our theme_exit() in oxygentheme.cpp. But theme_exit() is empty. I think all the destruction should be done there.

Created attachment 60938 [details]
attempted patch

Hello Ruslan,
I had similar issues in the past, but somehow don't have them anymore.
Anyway, your analysis about theme_exit() sounds convincing. 
I left it empty in the past, because it is actually not called (ever) for 90% of the apps. So I got lazy.

The attached patch deletes oxygen::Style::Intance() in theme_exit, and contains some safety belt in case the deletion should not have happened (namely, the singleton gets reset to 0, so that everything gets "re-initialized" if the instance is attempted to be re-used after theme_exit is called.

Deleting Style::instance() should be enough to have everything cleaned-up (that's why I always tried to keep all objects as children of this guy in the past ;)). Tell me if 
- it fixes the issue for you,  
- you don't see any suspicious behavior with other apps (I have not as far as I've tested)
- and feel free to commit + close the bugs, if the above two are fulfilled.

Cheers,

Hugo

No, your patch doesn't fix this bug.
Also, now i've found what part of code gets called when the library is already unloaded. It's InnerShadowData::targetExposeEvent(). But i currently don't understand why it doesn't get disconnected in time...

On the other hand, disabling inner shadow hack doesn't remove this bug. It instead leads to even less informative backtrace:

Program received signal SIGSEGV, Segmentation fault.
[Switching to Thread 0xb6fa5800 (LWP 26524)]
0xb763401b in g_main_context_prepare (context=0x8071c00, priority=0xbffff39c) at gmain.c:2757
2757              prepare = source->source_funcs->prepare;
(gdb) bt
#0  0xb763401b in g_main_context_prepare (context=0x8071c00, priority=0xbffff39c) at gmain.c:2757
#1  0xb7634d07 in g_main_context_iterate (context=0x8071c00, block=1, dispatch=1, self=0x80534d0) at gmain.c:3071
#2  0xb763568a in g_main_loop_run (loop=0x8654010) at gmain.c:3299
#3  0xb7cbd7a5 in gtk_main () from /usr/lib/libgtk-x11-2.0.so.0
#4  0x0804d4ba in main (argc=1, argv=0xbffff524) at main.c:231

I suspect this in fact might be GTK bug... still have to understand how to reduce the case.

New strange facts: commenting out the block of code from gdk_screen_get_default() to "} else" which leaves only image surface creation still leads to crash.
At the same time, creating a GtkWindow (and even just g_object_newv(GTK_TYPE_WINDOW,0,NULL)) somewhere in this code prevents crash.

Git commit 8ddab0bfb6a7377b28206cbc1463b8aa8c08a552 by Hugo Pereira Da Costa.
Committed on 07/03/2012 at 15:07.
Pushed by hpereiradacosta into branch 'master'.

Make sure signals and hooks are all properly disconnected at deletion of Oxygen::Style::Instance
Delete TimeLine::Server in theme_exit.

Addresses (partially) https://bugs.mageia.org/show_bug.cgi?id=2679
To my understanding, there is still a crash due to DBus (still working on it),
and issues with gtk_window_set_composite (reverting to old value does not seem to work. Might be a GTk bug).

M  +11   -0    src/animations/oxygenanimations.cpp
M  +1    -1    src/animations/oxygencomboboxdata.h
M  +1    -1    src/animations/oxygencomboboxentrydata.cpp
M  +1    -1    src/animations/oxygencomboboxentrydata.h
M  +21   -1    src/animations/oxygenhook.cpp
M  +2    -2    src/animations/oxygenhoverdata.cpp
M  +1    -1    src/animations/oxygenhoverdata.h
M  +3    -3    src/animations/oxygeninnershadowdata.cpp
M  +5    -2    src/animations/oxygeninnershadowdata.h
M  +1    -1    src/animations/oxygenmainwindowdata.h
M  +1    -1    src/animations/oxygenmenubarstatedata.cpp
M  +1    -1    src/animations/oxygenmenubarstatedata.h
M  +1    -1    src/animations/oxygenmenustatedata.h
M  +4    -1    src/animations/oxygenpaneddata.h
M  +1    -1    src/animations/oxygenscrollbardata.cpp
M  +1    -1    src/animations/oxygenscrollbardata.h
M  +1    -1    src/animations/oxygenscrolledwindowdata.cpp
M  +1    -1    src/animations/oxygenscrolledwindowdata.h
M  +20   -1    src/animations/oxygensignal.cpp
M  +1    -1    src/animations/oxygentabwidgetdata.h
M  +13   -1    src/animations/oxygentimelineserver.cpp
M  +1    -1    src/animations/oxygentoolbarstatedata.h
M  +1    -1    src/animations/oxygentreeviewdata.cpp
M  +1    -1    src/animations/oxygentreeviewdata.h
M  +9    -1    src/oxygenargbhelper.cpp
M  +17   -0    src/oxygendbus.cpp
M  +2    -4    src/oxygendbus.h
M  +2    -4    src/oxygenshadowhelper.cpp
M  +13   -1    src/oxygenstylehelper.cpp
M  +1    -2    src/oxygenstylehelper.h
M  +3    -0    src/oxygentheme.cpp
M  +8    -0    src/oxygenwindowmanager.cpp

http://commits.kde.org/oxygen-gtk/8ddab0bfb6a7377b28206cbc1463b8aa8c08a552

In fact, as I've now tested, DBus doesn't affect this bug. It's enough to disable inner shadows hack to have this working, so inner shadows seem to be the only problem (at least for gtk-chtheme).

Oops, I was wrong. In fact, the following is true:
1. If I disable only DBus, there's no crash, but treeview is no longer drawn
2. Disabling both DBus and inner shadows hack makes everything work

Now after commit fe399f97bd478a33241a5a4213498072292ce841, inner shadows problem is solved, and now only DBus usage bug remains.

mmm Thinking again, the commit 8ddab0bfb6a7377b28206cbc1463b8aa8c08a552
Might have introduced a number of regressions (basically, signal being disconnected while inserted in the map). I need to double-check.

For your observation about the interplay with dbus and inner-shadow-hack I can reproduce the same observations as you. The dbus issue might be a bug in the glib-dbus library.
I've read that dbus support is now built-in glib starting from version 2.26, so I guess I'll try this instead, and disable dbus for too old versions of glib.

And, thanks for the fix for the inner-shadow-hack !
(hard to spot typo)

PS: I'd like to have this fixed and backported before releasing new versions of 1.2 (and gtk3), due to the mageia bug. At least if fix becomes available within ~1 week.

ok. After double checking, my changes are safe.
I'll merge, and focus on dbus.

Git commit cf3628cfdbb5eed04aba5d7ff9894c2c5544fe5c by Hugo Pereira Da Costa.
Committed on 08/03/2012 at 14:30.
Pushed by hpereiradacosta into branch '1.2'.

Make sure signals and hooks are all properly disconnected at deletion of Oxygen::Style::Instance
Delete TimeLine::Server in theme_exit.

Addresses (partially) https://bugs.mageia.org/show_bug.cgi?id=2679
To my understanding, there is still a crash due to DBus (still working on it),
and issues with gtk_window_set_composite (reverting to old value does not seem to work. Might be a GTk bug).

Conflicts:

	src/animations/oxygeninnershadowdata.cpp

M  +11   -0    src/animations/oxygenanimations.cpp
M  +1    -1    src/animations/oxygencomboboxdata.h
M  +1    -1    src/animations/oxygencomboboxentrydata.cpp
M  +1    -1    src/animations/oxygencomboboxentrydata.h
M  +21   -1    src/animations/oxygenhook.cpp
M  +2    -2    src/animations/oxygenhoverdata.cpp
M  +1    -1    src/animations/oxygenhoverdata.h
M  +2    -2    src/animations/oxygeninnershadowdata.cpp
M  +5    -2    src/animations/oxygeninnershadowdata.h
M  +1    -1    src/animations/oxygenmainwindowdata.h
M  +1    -1    src/animations/oxygenmenubarstatedata.cpp
M  +1    -1    src/animations/oxygenmenubarstatedata.h
M  +1    -1    src/animations/oxygenmenustatedata.h
M  +4    -1    src/animations/oxygenpaneddata.h
M  +1    -1    src/animations/oxygenscrollbardata.cpp
M  +1    -1    src/animations/oxygenscrollbardata.h
M  +1    -1    src/animations/oxygenscrolledwindowdata.cpp
M  +1    -1    src/animations/oxygenscrolledwindowdata.h
M  +20   -1    src/animations/oxygensignal.cpp
M  +1    -1    src/animations/oxygentabwidgetdata.h
M  +13   -1    src/animations/oxygentimelineserver.cpp
M  +1    -1    src/animations/oxygentoolbarstatedata.h
M  +1    -1    src/animations/oxygentreeviewdata.cpp
M  +1    -1    src/animations/oxygentreeviewdata.h
M  +9    -1    src/oxygenargbhelper.cpp
M  +17   -0    src/oxygendbus.cpp
M  +2    -4    src/oxygendbus.h
M  +2    -4    src/oxygenshadowhelper.cpp
M  +13   -1    src/oxygenstylehelper.cpp
M  +1    -2    src/oxygenstylehelper.h
M  +3    -0    src/oxygentheme.cpp
M  +8    -0    src/oxygenwindowmanager.cpp

http://commits.kde.org/oxygen-gtk/cf3628cfdbb5eed04aba5d7ff9894c2c5544fe5c

Git commit f34a6758417ef5bc4fb51d20ce6f6ff8c8032bd8 by Hugo Pereira Da Costa.
Committed on 08/03/2012 at 15:49.
Pushed by hpereiradacosta into branch 'master'.

implement use of glib's build-in dbus support if glib version is new enough (2.26).
Fallback to old glib-dbus implementation, if fails.

M  +21   -9    CMakeLists.txt
M  +1    -0    config.h.cmake
M  +119  -44   src/oxygendbus.cpp
M  +38   -22   src/oxygendbus.h
M  +1    -1    src/oxygenstylewrapper.cpp
M  +1    -0    src/oxygentheme.cpp

http://commits.kde.org/oxygen-gtk/f34a6758417ef5bc4fb51d20ce6f6ff8c8032bd8

Latest commit, if glib is recent enough, fixes the crash for me.
(gtk-chtheme still crashes every now and then when swithcing theme +a lot+, but I am not sure it is our fault anymore).

@Ruslan, please confirm, and double check.

I also checked (by artificially increasing the required glib version number in CMakeLists.txt), that the old implementation still works, and still crashes.

Git commit ac6c14cfedafdb08ffe8f0d00febb56e62a65e82 by Hugo Pereira Da Costa.
Committed on 09/03/2012 at 11:56.
Pushed by hpereiradacosta into branch 'gtk3'.

moved disconnect call outside of destructor of Oxygen::DBus
call disconnect explicitely in theme_exit()
comment out disconnect call, to prevent oxygen-gtk-chtheme crash.

M  +1    -4    src/oxygendbus.cpp
M  +3    -0    src/oxygentheme.cpp

http://commits.kde.org/oxygen-gtk/ac6c14cfedafdb08ffe8f0d00febb56e62a65e82

@Ruslan,
1/ I was able to reproduce crashes even after commits from #13, especially when running with gdb.
Seems like there is some serious memory corruption when disconnecting the dbus interface.

2/ would you double-check that after commit from #15 the crash is gone ? 
Here it is gone (both running with and without gdb), although sadly enough there are other issues, namely:
if you switch away from oxygen and back to oxygen, then any update on the settings will crash, because of the unclear dangling connection ...

Still. Keep me posted.
PS: I have also merged with gtk3.
If you confirm that crash is gone, I'll merge with 1.2 and gtk3-1.0 since the current situation is already some kind of improvement with respect to before ...

(In reply to comment #16)
> 2/ would you double-check that after commit from #15 the crash is gone ? 
Yes, the basic crash is gone, i.e. when you start gtk-chtheme and immediately change theme
> if you switch away from oxygen and back to oxygen, then any update on the
> settings will crash, because of the unclear dangling connection ...
Strictly this way doesn't crash for me, but when I change some settings, and *then* change from oxygen to another style, it does crash. But not always. Sometimes something resembling your description also occurs, but all this is not regularly reproducible.

ok. I'll merge to the release branches and will release today.