Merge pull request #5174 from Alexays/fix-5051

fix(image): don't crash at startup when the widget isn't realized yet (#5051)

include/modules/image.hpp

@@ -19,6 +19,9 @@ namespace image {
 class IStrategy {
  public:
   virtual ~IStrategy() = default;
+  // Runs on the worker thread before update(). Use it for blocking work (e.g.
+  // spawning a user script) so the GTK main loop isn't stalled. Default no-op.
+  virtual void fetch() {}
   virtual void update() = 0;
 };
 
@@ -46,6 +49,7 @@ class MultipleImageStrategy : public IStrategy {
  public:
   MultipleImageStrategy(const std::string&, const Json::Value&, const std::string&, Gtk::EventBox&);
   ~MultipleImageStrategy() override = default;
+  void fetch() override;
   void update() override;
 
  private:
@@ -67,6 +71,8 @@ class MultipleImageStrategy : public IStrategy {
   int size_;
   Gtk::Box box_;
   std::vector<ImageData> images_data_;
+  // stdout captured by fetch() on the worker thread and consumed by update()
+  std::string exec_output_;
 };
 
 }  // namespace image

src/modules/image.cpp

@@ -45,6 +45,9 @@ auto waybar::modules::Image::getStrategy(const std::string& id, const Json::Valu
 
 void waybar::modules::Image::delayWorker() {
   thread_ = [this] {
+    // Do the blocking work (e.g. running a user script) here on the worker
+    // thread; update() then only parses the result and draws on the main thread.
+    strategy_->fetch();
     dp.emit();
     thread_.sleep_for(interval_);
   };
@@ -82,6 +85,15 @@ MultipleImageStrategy::MultipleImageStrategy(const std::string& id, const Json::
   }
 }
 
+void MultipleImageStrategy::fetch() {
+  // Run the (blocking) user script off the GTK main thread so the bar doesn't
+  // freeze for the script's duration on every interval. update() consumes the
+  // captured output. The static "entries" path takes priority and needs no exec.
+  if (config_["entries"].empty() && !config_["exec"].empty()) {
+    exec_output_ = util::command::exec(config_["exec"].asString(), "").out;
+  }
+}
+
 void MultipleImageStrategy::update() {
   // spdlog::info("update function run");
 
@@ -94,12 +106,12 @@ void MultipleImageStrategy::update() {
   if (!config_["entries"].empty()) {
     setImagesData(config_["entries"]);
   } else if (!config_["exec"].empty()) {
-    auto exec = util::command::exec(config_["exec"].asString(), "");
+    // exec output was captured by fetch() on the worker thread
     Json::Value as_json;
     Json::Reader reader;
 
-    if (!reader.parse(exec.out, as_json)) {
+    if (!reader.parse(exec_output_, as_json)) {
-      spdlog::error("invalid json from exec {}", exec.out);
+      spdlog::error("invalid json from exec {}", exec_output_);
       return;
     }
 
@@ -211,9 +223,8 @@ void MultipleImageStrategy::resetBoxAndMemory() {
 }
 
 void MultipleImageStrategy::handleClick(const Glib::ustring& data) {
-  auto msg = std::string(data);
+  // Fire-and-forget: don't block the main loop waiting on the command's output.
-
+  util::command::forkExec(data);
-  auto exec = util::command::exec(data, "");
 }
 
 SingleImageStrategy::SingleImageStrategy(const std::string& id, const Json::Value& config,
@@ -265,9 +276,19 @@ void SingleImageStrategy::update() {
   }
 
   if (pixbuf) {
-    auto surface = Gdk::Cairo::create_surface_from_pixbuf(pixbuf, image_.get_scale_factor(),
+    // Building a HiDPI-aware cairo surface requires a realized widget: it reads the
-                                                          image_.get_window());
+    // GdkWindow and its scale factor. During startup update() can run before the
-    image_.set(surface);
+    // Gtk::Image is realized, in which case get_window() is null; feeding that path a
+    // null window aborts startup. Fall back to setting the pixbuf directly while the
+    // widget is unrealized, and use the crisp surface path once a window is available.
+    auto window = image_.get_window();
+    if (window) {
+      auto surface =
+          Gdk::Cairo::create_surface_from_pixbuf(pixbuf, image_.get_scale_factor(), window);
+      image_.set(surface);
+    } else {
+      image_.set(pixbuf);
+    }
     image_.show();
 
     if (hasTooltip_ && !tooltip_.empty()) {