Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AppIndicators_IconActor._getIconInfo() returns NULL causing an exception #245

Closed
delxg opened this issue Jun 26, 2020 · 1 comment
Closed

Comments

@delxg
Copy link

delxg commented Jun 26, 2020

Hi!

System details:

  • Ubuntu 20.04, fully up to date
  • gnome-shell on wayland
  • appindicators v33

Problem summary

Using syncthing-gtk sometimes causes gnome-shell to crash

Problem analysis

I see this error in the log:

syncthing-gtk[171577]: gtk_widget_get_scale_factor: assertion 'GTK_IS_WIDGET (widget)' failed
syncthing-gtk[171577]: gtk_widget_get_scale_factor: assertion 'GTK_IS_WIDGET (widget)' failed
syncthing-gtk[171577]: gtk_widget_get_scale_factor: assertion 'GTK_IS_WIDGET (widget)' failed
syncthing-gtk[171577]: gtk_widget_get_scale_factor: assertion 'GTK_IS_WIDGET (widget)' failed
syncthing-gtk[171577]: gtk_widget_get_scale_factor: assertion 'GTK_IS_WIDGET (widget)' failed
syncthing-gtk.desktop[171577]: I App           updatecheck: disabled
gnome-shell[171339]: gtk_icon_theme_get_for_screen: assertion 'GDK_IS_SCREEN (screen)' failed
gnome-shell[171339]: JS ERROR: TypeError: src is null
                     connectSmart4A@/usr/share/gnome-shell/extensions/[email protected]/util.js:189:21
                     connectSmart@/usr/share/gnome-shell/extensions/[email protected]/util.js:214:31
                     _init@/usr/share/gnome-shell/extensions/[email protected]/appIndicator.js:325:14
                     _init@/usr/share/gnome-shell/extensions/[email protected]/indicatorStatusIcon.js:42:25
                     _registerItem@/usr/share/gnome-shell/extensions/[email protected]/statusNotifierWatcher.js:95:22
                     _ensureItemRegistered@/usr/share/gnome-shell/extensions/[email protected]/statusNotifierWatcher.js:123:14
                     RegisterStatusNotifierItemAsync@/usr/share/gnome-shell/extensions/[email protected]/statusNotifierWatcher.js:169:14
                     _handleMethodCall@resource:///org/gnome/gjs/modules/core/overrides/Gio.js:371:35
                     _wrapJSObject/<@resource:///org/gnome/gjs/modules/core/overrides/Gio.js:404:34
gnome-shell[171339]: gtk_icon_theme_get_for_screen: assertion 'GDK_IS_SCREEN (screen)' failed
gnome-shell[171339]: JS ERROR: Exception in callback for signal: icon: Error: Argument 'filename' (type filename) may not be null
                     _createIconByName@/usr/share/gnome-shell/extensions/[email protected]/appIndicator.js:406:26
                     _cacheOrCreateIconByName@/usr/share/gnome-shell/extensions/[email protected]/appIndicator.js:368:14
                     _updateIconByType@/usr/share/gnome-shell/extensions/[email protected]/appIndicator.js:574:18
                     _updateIcon@/usr/share/gnome-shell/extensions/[email protected]/appIndicator.js:601:14
                     _emit@resource:///org/gnome/gjs/modules/core/_signals.js:133:47
                     _onPropertiesChanged/<@/usr/share/gnome-shell/extensions/[email protected]/appIndicator.js:220:22
                     _onPropertiesChanged@/usr/share/gnome-shell/extensions/[email protected]/appIndicator.js:214:15

It seems to me like _createIconByName() was called with a path === null, which causes an assertion to fail inside gdk-pixbuf. This ultimately crashes the shell.

There are a couple of ways that _getIconInfo() could return null:

  1. It could be called with name === null
  2. If lookup_icon_for_scale() returns null then a warning will be logged and _getIconInfo() will return null.
  3. If Gtk.IconTheme.get_default() returns null then _getIconInfo() will return null with no logging.

I don't believe (1) is happening because it's called from _cacheOrCreateIconByName() and the check on line 573 guards against this. It's not (2) because I'm not seeing that warning in the log.

By process of elimination I believe it must be (3).

Looking at the docs for gtk_icon_theme_get_default it doesn't say whether it returns null, and I would have assumed it cannot. However checking the implementation in gtkicontheme.c:451 shows this:

GtkIconTheme *
gtk_icon_theme_get_default (void)
{
  return gtk_icon_theme_get_for_screen (gdk_screen_get_default ());
}

GtkIconTheme *
gtk_icon_theme_get_for_screen (GdkScreen *screen)
{
  GtkIconTheme *icon_theme;

  g_return_val_if_fail (GDK_IS_SCREEN (screen), NULL);
  /* snip */
}

Note that gdk_screen_get_default can return null sometimes.

This is supported by the assertion failure in the log I posted:

gtk_icon_theme_get_for_screen: assertion 'GDK_IS_SCREEN (screen)' failed

Conclusion

So in conclusion I think gtk_icon_theme_get_default() is returning null occasionally, causing AppIndicators_IconActor._getIconInfo() to return null, causing GdkPixbuf.Pixbuf.get_file_info_async() to fail an assertion and crash the gnome-shell.

A possible fix is in #243, but it may be nice to look at a few other fixes:

  • Could gtk_icon_theme_get_default() be made to never return null?
  • Or possibly the docs for gtk_icon_theme_get_default() should be updated to make it clear that it can return null in some cases.
  • Are there any other places where gtk_icon_theme_get_default() is assumed to never return null that could be causing other bugs/crashes?

Related bugs

This seems a little related to some other bug reports: #241, #236, #226. In those three _getIconInfo() appears to be returning null due to branch (2) above because it is logging a warning.

Thanks!

I hope this bug report was helpful. Thanks for your time.

Great work on Ubuntu and related gnome-shell extensions! :)

@3v1n0
Copy link
Collaborator

3v1n0 commented Sep 17, 2020

Closed as by commit 745c66a

@3v1n0 3v1n0 closed this as completed Sep 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants