Merge pull request #2277 from Aparicio99/fix_icon_theme_segfault

include/util/gtk_icon.hpp

@@ -0,0 +1,12 @@
+#pragma once
+#include <string>
+#include <mutex>
+#include <gtkmm/icontheme.h>
+
+class DefaultGtkIconThemeWrapper {
+  private:
+    static std::mutex default_theme_mutex;
+  public:
+    static bool has_icon(const std::string&);
+    static Glib::RefPtr<Gdk::Pixbuf> load_icon(const char*, int, Gtk::IconLookupFlags);
+};

meson.build

@@ -173,7 +173,8 @@ src_files = files(
     'src/util/prepare_for_sleep.cpp',
     'src/util/ustring_clen.cpp',
     'src/util/sanitize_str.cpp',
-    'src/util/rewrite_string.cpp'
+    'src/util/rewrite_string.cpp',
+    'src/util/gtk_icon.cpp'
 )
 
 inc_dirs = ['include']

src/modules/gamemode.cpp

@@ -16,10 +16,11 @@
 #include "glibmm/ustring.h"
 #include "glibmm/variant.h"
 #include "glibmm/varianttype.h"
-#include "gtkmm/icontheme.h"
 #include "gtkmm/label.h"
 #include "gtkmm/tooltip.h"
 
+#include "util/gtk_icon.hpp"
+
 namespace waybar::modules {
 Gamemode::Gamemode(const std::string& id, const Json::Value& config)
     : AModule(config, "gamemode", id), box_(Gtk::ORIENTATION_HORIZONTAL, 0), icon_(), label_() {
@@ -224,7 +225,7 @@ auto Gamemode::update() -> void {
   label_.set_markup(str);
 
   if (useIcon) {
-    if (!Gtk::IconTheme::get_default()->has_icon(iconName)) {
+    if (!DefaultGtkIconThemeWrapper::has_icon(iconName)) {
       iconName = DEFAULT_ICON_NAME;
     }
     icon_.set_from_icon_name(iconName, Gtk::ICON_SIZE_INVALID);

src/modules/sni/item.cpp

@@ -9,6 +9,7 @@
 #include <map>
 
 #include "util/format.hpp"
+#include "util/gtk_icon.hpp"
 
 template <>
 struct fmt::formatter<Glib::VariantBase> : formatter<std::string> {
@@ -379,10 +380,8 @@ Glib::RefPtr<Gdk::Pixbuf> Item::getIconByName(const std::string& name, int reque
     return icon_theme->load_icon(name.c_str(), tmp_size,
                                  Gtk::IconLookupFlags::ICON_LOOKUP_FORCE_SIZE);
   }
-  Glib::RefPtr<Gtk::IconTheme> default_theme = Gtk::IconTheme::get_default();
+  return DefaultGtkIconThemeWrapper::load_icon(name.c_str(), tmp_size,
-  default_theme->rescan_if_needed();
+                                               Gtk::IconLookupFlags::ICON_LOOKUP_FORCE_SIZE);
-  return default_theme->load_icon(name.c_str(), tmp_size,
-                                  Gtk::IconLookupFlags::ICON_LOOKUP_FORCE_SIZE);
 }
 
 double Item::getScaledIconSize() {

src/modules/sway/window.cpp

@@ -5,7 +5,6 @@
 #include <glibmm/keyfile.h>
 #include <glibmm/miscutils.h>
 #include <gtkmm/enums.h>
-#include <gtkmm/icontheme.h>
 #include <spdlog/spdlog.h>
 
 #include <filesystem>
@@ -13,6 +12,7 @@
 #include <string>
 
 #include "util/rewrite_string.hpp"
+#include "util/gtk_icon.hpp"
 
 namespace waybar::modules::sway {
 
@@ -81,13 +81,12 @@ std::optional<Glib::ustring> getIconName(const std::string& app_id, const std::s
   if (!desktop_file_path.has_value()) {
     // Try some heuristics to find a matching icon
 
-    const auto default_icon_theme = Gtk::IconTheme::get_default();
+    if (DefaultGtkIconThemeWrapper::has_icon(app_id)) {
-    if (default_icon_theme->has_icon(app_id)) {
       return app_id;
     }
 
     const auto app_id_desktop = app_id + "-desktop";
-    if (default_icon_theme->has_icon(app_id_desktop)) {
+    if (DefaultGtkIconThemeWrapper::has_icon(app_id_desktop)) {
       return app_id_desktop;
     }
 
@@ -101,7 +100,7 @@ std::optional<Glib::ustring> getIconName(const std::string& app_id, const std::s
     const auto first_space = app_id.find_first_of(' ');
     if (first_space != std::string::npos) {
       const auto first_word = to_lower(app_id.substr(0, first_space));
-      if (default_icon_theme->has_icon(first_word)) {
+      if (DefaultGtkIconThemeWrapper::has_icon(first_word)) {
         return first_word;
       }
     }
@@ -109,7 +108,7 @@ std::optional<Glib::ustring> getIconName(const std::string& app_id, const std::s
     const auto first_dash = app_id.find_first_of('-');
     if (first_dash != std::string::npos) {
       const auto first_word = to_lower(app_id.substr(0, first_dash));
-      if (default_icon_theme->has_icon(first_word)) {
+      if (DefaultGtkIconThemeWrapper::has_icon(first_word)) {
         return first_word;
       }
     }

src/modules/upower/upower.cpp

@@ -5,9 +5,10 @@
 #include <cstring>
 #include <string>
 
-#include "gtkmm/icontheme.h"
 #include "gtkmm/tooltip.h"
 
+#include "util/gtk_icon.hpp"
+
 namespace waybar::modules::upower {
 UPower::UPower(const std::string& id, const Json::Value& config)
     : AModule(config, "upower", id),
@@ -372,7 +373,7 @@ auto UPower::update() -> void {
   label_.set_markup(onlySpaces ? "" : label_format);
 
   // Set icon
-  if (icon_name == NULL || !Gtk::IconTheme::get_default()->has_icon(icon_name)) {
+  if (icon_name == NULL || !DefaultGtkIconThemeWrapper::has_icon(icon_name)) {
     icon_name = (char*)"battery-missing-symbolic";
   }
   icon_.set_from_icon_name(icon_name, Gtk::ICON_SIZE_INVALID);

src/modules/upower/upower_tooltip.cpp

@@ -2,10 +2,11 @@
 
 #include "gtkmm/box.h"
 #include "gtkmm/enums.h"
-#include "gtkmm/icontheme.h"
 #include "gtkmm/image.h"
 #include "gtkmm/label.h"
 
+#include "util/gtk_icon.hpp"
+
 namespace waybar::modules::upower {
 UPowerTooltip::UPowerTooltip(uint iconSize_, uint tooltipSpacing_, uint tooltipPadding_)
     : Gtk::Window(),
@@ -62,7 +63,7 @@ uint UPowerTooltip::updateTooltip(Devices& devices) {
     std::string deviceIconName = getDeviceIcon(kind);
     Gtk::Image* deviceIcon = new Gtk::Image();
     deviceIcon->set_pixel_size(iconSize);
-    if (!Gtk::IconTheme::get_default()->has_icon(deviceIconName)) {
+    if (!DefaultGtkIconThemeWrapper::has_icon(deviceIconName)) {
       deviceIconName = "battery-missing-symbolic";
     }
     deviceIcon->set_from_icon_name(deviceIconName, Gtk::ICON_SIZE_INVALID);
@@ -79,7 +80,7 @@ uint UPowerTooltip::updateTooltip(Devices& devices) {
     // Set icon
     Gtk::Image* icon = new Gtk::Image();
     icon->set_pixel_size(iconSize);
-    if (icon_name == NULL || !Gtk::IconTheme::get_default()->has_icon(icon_name)) {
+    if (icon_name == NULL || !DefaultGtkIconThemeWrapper::has_icon(icon_name)) {
       icon_name = (char*)"battery-missing-symbolic";
     }
     icon->set_from_icon_name(icon_name, Gtk::ICON_SIZE_INVALID);

src/util/gtk_icon.cpp

@@ -0,0 +1,26 @@
+#include "util/gtk_icon.hpp"
+
+/* We need a global mutex for accessing the object returned by Gtk::IconTheme::get_default()
+ * because it always returns the same object across different threads, and concurrent
+ * access can cause data corruption and lead to invalid memory access and crashes.
+ * Even concurrent calls that seem read only such as has_icon can cause issues because
+ * the GTK lib may update the internal icon cache on this calls.
+*/
+
+std::mutex DefaultGtkIconThemeWrapper::default_theme_mutex;
+
+bool DefaultGtkIconThemeWrapper::has_icon(const std::string& value) {
+
+  const std::lock_guard<std::mutex> lock(default_theme_mutex);
+
+  return Gtk::IconTheme::get_default()->has_icon(value);
+}
+
+Glib::RefPtr<Gdk::Pixbuf> DefaultGtkIconThemeWrapper::load_icon(const char *name, int tmp_size, Gtk::IconLookupFlags flags) {
+
+  const std::lock_guard<std::mutex> lock(default_theme_mutex);
+
+  auto default_theme = Gtk::IconTheme::get_default();
+  default_theme->rescan_if_needed();
+  return default_theme->load_icon(name, tmp_size, flags);
+}