GNOME Bugzilla – Bug 692845
shell crash after use switching
Last modified: 2013-02-14 21:41:17 UTC
I have been seeing a pretty regular shell crash lately in jhbuild. I usually see it after fast user switching. ** Gtk:ERROR:gtkicontheme.c:3526:proxy_symbolic_pixbuf_destroy: assertion failed: (symbolic_cache != NULL) gnome-session[2577]: WARNING: Application 'gnome-shell.desktop' killed by signal 6 ** Message: applet now removed from the notification area
I've seen this too. I believe this is a race condition in GtkIconTheme, uncovered when SVG loading became threadsafe (and thus unlocked). I haven't had the chance to debug or fix it, though.
Ok, so I looked at GtkIconTheme a bit, and I don't see an easy way to make it threadsafe. It should be possible to make icon loading async in Gtk, though. I'll open a Gtk bug.
no gtk api has ever been threadsafe, including gtk_icon_theme_... you are supposed to take the gdk lock when calling it from other threads
But that defeats the point of using more threads, because then you are completely serialized.
We don't use gtk_icon_theme API on any thread but the main thread, as far as I can tell. It's the pixbuf loader API that actually loads the image that should be threadsafe.
No, we call gtk_icon_info_load_icon (load_symbolic), from impl_load_pixbuf_gicon, which is called from load_pixbuf_thread, itself called from a threaded GSimpleAsyncResult.
As far as I'm aware, that simply loads the file using GdkPixbuf, which should be thread-safe.
Nope, it frobs the GtkIconInfo structure to cache the loaded GdkPixbuf, and that's not threadsafe (in particular, there is a lot of caching which involves symbolic icons, so the window for a crash becomes larger there)
*** Bug 693722 has been marked as a duplicate of this bug. ***
Created attachment 236155 [details] [review] st-texture-cache: Remove a no-op scale pixbuf We already passed GtkIconTheme the size when it wanted to load it, so it knows the desired size and will scale it to what we want.
Created attachment 236156 [details] [review] st-texture-cache: Add one simple way to free texture load data
Created attachment 236157 [details] [review] st-texture-cache: Use async variants of the icon loading API While we were relying on gtk_icon_info_load_icon and friends being thread-safe, there was no such guarantee, and recent caching that was added to GTK+ made it non-threadsafe. To replace it, _async() variants of the icon loading code were added that are thread-safe. Use those instead of using our own worker threads.
Review of attachment 236155 [details] [review]: Sure.
Review of attachment 236156 [details] [review]: Definitely. Although while you're there, you should probably use GSlice for the structure.
Review of attachment 236156 [details] [review]: Sure.
Created attachment 236164 [details] [review] st-texture-cache: Use async variants of the icon loading API While we were relying on gtk_icon_info_load_icon and friends being thread-safe, there was no such guarantee, and recent caching that was added to GTK+ made it non-threadsafe. To replace it, _async() variants of the icon loading code were added that are thread-safe. Use those instead of using our own worker threads. Clean up the patch; make the error case do what it did before (remove the outstanding request, etc.)
Review of attachment 236164 [details] [review]: Tested locally, looks good to me.
Attachment 236155 [details] pushed as f1cdce3 - st-texture-cache: Remove a no-op scale pixbuf Attachment 236156 [details] pushed as a5d3f77 - st-texture-cache: Add one simple way to free texture load data Attachment 236164 [details] pushed as 7fc2b33 - st-texture-cache: Use async variants of the icon loading API