From 4dfb2370a454159c2814406fd59e1e84d44833aa Mon Sep 17 00:00:00 2001 From: Melroy van den Berg Date: Tue, 21 May 2024 16:44:04 +0200 Subject: [PATCH 1/4] Version check should be done in a separate thread --- include/main_window.h | 29 ++++++- include/signal_controller.h | 2 +- src/main_window.cc | 165 +++++++++++++++++++++++++++++++----- src/signal_controller.cc | 2 - 4 files changed, 171 insertions(+), 27 deletions(-) diff --git a/include/main_window.h b/include/main_window.h index 8ea5433..a0e53f0 100644 --- a/include/main_window.h +++ b/include/main_window.h @@ -31,6 +31,7 @@ #include #include #include +#include using std::cout; using std::endl; @@ -91,10 +92,17 @@ class MainWindow : public Gtk::Window protected: // Signal handlers void on_startup_version_update(); - bool delete_window(GdkEventAny* any_event); - Glib::RefPtr window_settings; /*!< Window settings to store our window settings, even during restarts */ + void on_error_message_check_version(); + void on_info_message_check_version(); + void on_new_version_available(); + + bool signal_delete_window(GdkEventAny* any_event); + void signal_error_message_check_version(); + void signal_info_message_check_version(); + void signal_new_version_available(); - AppListModelColumns app_list_columns; /*!< Application list model columns for app tree view */ + Glib::RefPtr window_settings; /*!< Window settings to store our window settings, even during restarts */ + AppListModelColumns app_list_columns; /*!< Application list model columns for app tree view */ // Child widgets Gtk::Box vbox; /*!< The main vertical box */ @@ -154,8 +162,19 @@ class MainWindow : public Gtk::Window // Busy dialog BusyDialog busy_dialog_; /*!< Busy dialog, when the user should wait until install is finished */ private: + mutable std::mutex info_message_mutex_; /*!< Synchronizes access to info message using mutex */ + mutable std::mutex error_message_mutex_; /*!< Synchronizes access to error message using mutex */ + mutable std::mutex new_version_mutex_; /*!< Synchronizes access to new version using mutex */ + Glib::ustring info_message_; + Glib::ustring error_message_; + string new_version_; BottleNewAssistant new_bottle_assistant_; /*!< New bottle wizard (behind the "new" toolbar button) */ GeneralConfigData general_config_data_; + std::thread* thread_check_version_; /*!< Thread for checking version */ + // Dispatchers for handling signals from the thread towards a GUI thread + Glib::Dispatcher error_message_check_version_dispatcher_; + Glib::Dispatcher info_message_check_version_dispatcher_; + Glib::Dispatcher new_version_available_dispatcher_; // Signal handlers virtual void on_bottle_row_clicked(Gtk::ListBoxRow* row); @@ -167,7 +186,9 @@ class MainWindow : public Gtk::Window void set_detailed_info(const BottleItem& bottle); void set_application_list(const string& prefix_path, const std::map& app_List); void add_application(const string& name, const string& description, const string& command, const string& icon_name, bool is_icon_full_path = false); - void check_version_update(bool show_equal = false); + void cleanup_check_version_thread(); + void check_version_update(bool show_equal_or_error = false); + void check_version(MainWindow* mainWindow, bool show_equal_or_error); void load_stored_window_settings(); void create_left_panel(); void create_right_panel(); diff --git a/include/signal_controller.h b/include/signal_controller.h index eaeef6d..3d611fe 100644 --- a/include/signal_controller.h +++ b/include/signal_controller.h @@ -103,7 +103,7 @@ class SignalController : public Gtk::Window AddAppWindow& add_app_window_; RemoveAppWindow& remove_app_window_; - // Dispatcher for handling signals from the thread towards a GUI thread + // Dispatchers for handling signals from the thread towards a GUI thread Glib::Dispatcher bottle_created_dispatcher_; Glib::Dispatcher bottle_updated_dispatcher_; Glib::Dispatcher bottle_cloned_dispatcher_; diff --git a/src/main_window.cc b/src/main_window.cc index 1187172..9045c68 100644 --- a/src/main_window.cc +++ b/src/main_window.cc @@ -43,7 +43,8 @@ MainWindow::MainWindow(Menu& menu) app_list_top_hbox(Gtk::Orientation::ORIENTATION_HORIZONTAL), container_paned(Gtk::Orientation::ORIENTATION_HORIZONTAL), separator1(Gtk::Orientation::ORIENTATION_HORIZONTAL), - busy_dialog_(*this) + busy_dialog_(*this), + thread_check_version_(nullptr) { // Set some Window properties set_title("WineGUI - WINE Manager"); @@ -118,10 +119,15 @@ MainWindow::MainWindow(Menu& menu) remove_app_list_button.signal_clicked().connect(show_remove_app_window); refresh_app_list_button.signal_clicked().connect(sigc::mem_fun(*this, &MainWindow::on_refresh_app_list_button_clicked)); + // Dispatch signals + error_message_check_version_dispatcher_.connect(sigc::mem_fun(this, &MainWindow::on_error_message_check_version)); + info_message_check_version_dispatcher_.connect(sigc::mem_fun(this, &MainWindow::on_info_message_check_version)); + new_version_available_dispatcher_.connect(sigc::mem_fun(this, &MainWindow::on_new_version_available)); + // Check for update (when GTK is idle) Glib::signal_idle().connect_once(sigc::mem_fun(*this, &MainWindow::on_startup_version_update), Glib::PRIORITY_DEFAULT_IDLE); // Window closed signal - signal_delete_event().connect(sigc::mem_fun(this, &MainWindow::delete_window)); + signal_delete_event().connect(sigc::mem_fun(this, &MainWindow::signal_delete_window)); // Show the widget children show_all_children(); @@ -132,6 +138,8 @@ MainWindow::MainWindow(Menu& menu) */ MainWindow::~MainWindow() { + // Avoid zombies + this->cleanup_check_version_thread(); } /** @@ -498,13 +506,62 @@ void MainWindow::on_new_bottle_apply() */ void MainWindow::on_startup_version_update() { - check_version_update(); // Without message when versions are equal + // Silent check + check_version_update(); +} + +/** + * \brief Show error messages from the version check thread + */ +void MainWindow::on_error_message_check_version() +{ + this->cleanup_check_version_thread(); + + { + std::lock_guard lock(error_message_mutex_); + show_error_message(error_message_); + } +} + +/** + * \brief Show info messages from the version check thread + */ +void MainWindow::on_info_message_check_version() +{ + this->cleanup_check_version_thread(); + + { + std::lock_guard lock(info_message_mutex_); + show_info_message(info_message_); + } +} + +/** + * \brief Show new version available dialog from the version check thread + */ +void MainWindow::on_new_version_available() +{ + this->cleanup_check_version_thread(); + + // Show dialog + { + std::lock_guard lock(new_version_mutex_); + string message = "New WineGUI release is out. Please, update WineGUI to the latest release.\n" + "You are using: v" + + std::string(PROJECT_VER) + ". Latest version: v" + new_version_ + "."; + Gtk::MessageDialog dialog(*this, message, true, Gtk::MESSAGE_WARNING, Gtk::BUTTONS_OK); + dialog.set_secondary_text("Download the latest release now!", + true); + dialog.set_title("New WineGUI Release!"); + dialog.set_modal(true); + dialog.run(); + } } /** * \brief Called when Window is closed/exited */ -bool MainWindow::delete_window(GdkEventAny* any_event __attribute__((unused))) +bool MainWindow::signal_delete_window(GdkEventAny* any_event __attribute__((unused))) { if (window_settings) { @@ -522,6 +579,30 @@ bool MainWindow::delete_window(GdkEventAny* any_event __attribute__((unused))) return false; } +/** + * \brief Signal error message during version check + */ +void MainWindow::signal_error_message_check_version() +{ + error_message_check_version_dispatcher_.emit(); +} + +/** + * \brief Signal info message during version check + */ +void MainWindow::signal_info_message_check_version() +{ + info_message_check_version_dispatcher_.emit(); +} + +/** + * \brief Signal for new available version (opens dialog) + */ +void MainWindow::signal_new_version_available() +{ + new_version_available_dispatcher_.emit(); +} + /** * \brief set the detailed info panel on the right * \param[in] bottle - Wine Bottle item object @@ -755,43 +836,87 @@ void MainWindow::add_application(const string& name, const string& description, } } +/** + * \brief Helper method for cleaning the manage thread. + */ +void MainWindow::cleanup_check_version_thread() +{ + if (thread_check_version_) + { + if (thread_check_version_->joinable()) + thread_check_version_->join(); + delete thread_check_version_; + thread_check_version_ = nullptr; + } +} + /** * \brief Check for WineGUI version, is there an update? - * \param show_equal Also show user message when the versions matches. + * \param show_equal_or_error Also show message when the versions matches or an error occurs. */ -void MainWindow::check_version_update(bool show_equal) +void MainWindow::check_version_update(bool show_equal_or_error) +{ + if (thread_check_version_) + { + if (show_equal_or_error) + { + show_error_message("WineGUI version check already running"); + } + } + else + { + // Start the check version thread + thread_check_version_ = new std::thread([this, show_equal_or_error] { check_version(this, show_equal_or_error); }); + } +} + +/** + * \brief Check WineGUI version (runs in thread) + * \param[in] mainWindow Main Window pointer, in order to signal back events + * \param[in] show_equal_or_error Also show message when the versions matches or an error occurs. + */ +void MainWindow::check_version(MainWindow* mainWindow, bool show_equal_or_error) { string version = Helper::open_file_from_uri("https://winegui.melroy.org/latest_release.txt"); // Remove new lines version.erase(std::remove(version.begin(), version.end(), '\n'), version.end()); if (!version.empty()) { - // Is there a different version? Show the user the message to update to the latest release. + // Is there a different version? Signal a new version available. if (version.compare(PROJECT_VER) != 0) { - string message = "New WineGUI release is out. Please, update WineGUI to the latest release.\n" - "You are using: v" + - std::string(PROJECT_VER) + ". Latest version: v" + version + "."; - Gtk::MessageDialog dialog(*this, message, true, Gtk::MESSAGE_WARNING, Gtk::BUTTONS_OK); - dialog.set_secondary_text("Download the latest release now!", - true); - dialog.set_title("New WineGUI Release!"); - dialog.set_modal(true); - dialog.run(); + { + std::lock_guard lock(new_version_mutex_); + new_version_ = version; + } + mainWindow->signal_new_version_available(); // Will eventually show a dialog } else { - if (show_equal) + if (show_equal_or_error) { - show_info_message("WineGUI release is up-to-date. Well done!"); + + { + std::lock_guard lock(info_message_mutex_); + info_message_ = "WineGUI release is up-to-date. Well done!"; + } + mainWindow->signal_info_message_check_version(); + // Should trigger: + // show_info_message("WineGUI release is up-to-date. Well done!"); } } } else { - if (show_equal) + if (show_equal_or_error) { - show_error_message("We could determine the latest WineGUI version. Try again later."); + { + std::lock_guard lock(error_message_mutex_); + error_message_ = "We could not determine the latest WineGUI version. Try again later."; + } + mainWindow->signal_error_message_check_version(); + // Should trigger: + // show_error_message("We could not determine the latest WineGUI version. Try again later."); } } } diff --git a/src/signal_controller.cc b/src/signal_controller.cc index c42513d..4210899 100644 --- a/src/signal_controller.cc +++ b/src/signal_controller.cc @@ -55,8 +55,6 @@ SignalController::SignalController(BottleManager& manager, configure_window_(configure_window), add_app_window_(add_app_window), remove_app_window_(remove_app_window), - bottle_created_dispatcher_(), - error_message_created_dispatcher_(), thread_bottle_manager_(nullptr) { // Nothing From 87f386bab6ddaa9801dcf6e382faabf8a0e8bcea Mon Sep 17 00:00:00 2001 From: Melroy van den Berg Date: Tue, 21 May 2024 16:55:19 +0200 Subject: [PATCH 2/4] Clean-up comments --- src/main_window.cc | 4 ---- 1 file changed, 4 deletions(-) diff --git a/src/main_window.cc b/src/main_window.cc index 9045c68..ff9e354 100644 --- a/src/main_window.cc +++ b/src/main_window.cc @@ -901,8 +901,6 @@ void MainWindow::check_version(MainWindow* mainWindow, bool show_equal_or_error) info_message_ = "WineGUI release is up-to-date. Well done!"; } mainWindow->signal_info_message_check_version(); - // Should trigger: - // show_info_message("WineGUI release is up-to-date. Well done!"); } } } @@ -915,8 +913,6 @@ void MainWindow::check_version(MainWindow* mainWindow, bool show_equal_or_error) error_message_ = "We could not determine the latest WineGUI version. Try again later."; } mainWindow->signal_error_message_check_version(); - // Should trigger: - // show_error_message("We could not determine the latest WineGUI version. Try again later."); } } } From 4e62ec221f5f785c397366030ac886cafdce38a4 Mon Sep 17 00:00:00 2001 From: Melroy van den Berg Date: Tue, 21 May 2024 17:51:30 +0200 Subject: [PATCH 3/4] Remove unnecessary signal wrappers. Also remove unnecessary main window pointer --- include/main_window.h | 9 +++------ src/main_window.cc | 44 ++++++++++++------------------------------- 2 files changed, 15 insertions(+), 38 deletions(-) diff --git a/include/main_window.h b/include/main_window.h index a0e53f0..8a3a679 100644 --- a/include/main_window.h +++ b/include/main_window.h @@ -95,11 +95,7 @@ class MainWindow : public Gtk::Window void on_error_message_check_version(); void on_info_message_check_version(); void on_new_version_available(); - - bool signal_delete_window(GdkEventAny* any_event); - void signal_error_message_check_version(); - void signal_info_message_check_version(); - void signal_new_version_available(); + bool on_delete_window(GdkEventAny* any_event); Glib::RefPtr window_settings; /*!< Window settings to store our window settings, even during restarts */ AppListModelColumns app_list_columns; /*!< Application list model columns for app tree view */ @@ -175,6 +171,7 @@ class MainWindow : public Gtk::Window Glib::Dispatcher error_message_check_version_dispatcher_; Glib::Dispatcher info_message_check_version_dispatcher_; Glib::Dispatcher new_version_available_dispatcher_; + Glib::Dispatcher check_version_finished_dispatcher_; // Signal handlers virtual void on_bottle_row_clicked(Gtk::ListBoxRow* row); @@ -188,7 +185,7 @@ class MainWindow : public Gtk::Window void add_application(const string& name, const string& description, const string& command, const string& icon_name, bool is_icon_full_path = false); void cleanup_check_version_thread(); void check_version_update(bool show_equal_or_error = false); - void check_version(MainWindow* mainWindow, bool show_equal_or_error); + void check_version(bool show_equal_or_error); void load_stored_window_settings(); void create_left_panel(); void create_right_panel(); diff --git a/src/main_window.cc b/src/main_window.cc index ff9e354..fa86f3d 100644 --- a/src/main_window.cc +++ b/src/main_window.cc @@ -123,11 +123,12 @@ MainWindow::MainWindow(Menu& menu) error_message_check_version_dispatcher_.connect(sigc::mem_fun(this, &MainWindow::on_error_message_check_version)); info_message_check_version_dispatcher_.connect(sigc::mem_fun(this, &MainWindow::on_info_message_check_version)); new_version_available_dispatcher_.connect(sigc::mem_fun(this, &MainWindow::on_new_version_available)); + check_version_finished_dispatcher_.connect(sigc::mem_fun(this, &MainWindow::cleanup_check_version_thread)); // Check for update (when GTK is idle) Glib::signal_idle().connect_once(sigc::mem_fun(*this, &MainWindow::on_startup_version_update), Glib::PRIORITY_DEFAULT_IDLE); // Window closed signal - signal_delete_event().connect(sigc::mem_fun(this, &MainWindow::signal_delete_window)); + signal_delete_event().connect(sigc::mem_fun(this, &MainWindow::on_delete_window)); // Show the widget children show_all_children(); @@ -561,7 +562,7 @@ void MainWindow::on_new_version_available() /** * \brief Called when Window is closed/exited */ -bool MainWindow::signal_delete_window(GdkEventAny* any_event __attribute__((unused))) +bool MainWindow::on_delete_window(GdkEventAny* any_event __attribute__((unused))) { if (window_settings) { @@ -579,30 +580,6 @@ bool MainWindow::signal_delete_window(GdkEventAny* any_event __attribute__((unus return false; } -/** - * \brief Signal error message during version check - */ -void MainWindow::signal_error_message_check_version() -{ - error_message_check_version_dispatcher_.emit(); -} - -/** - * \brief Signal info message during version check - */ -void MainWindow::signal_info_message_check_version() -{ - info_message_check_version_dispatcher_.emit(); -} - -/** - * \brief Signal for new available version (opens dialog) - */ -void MainWindow::signal_new_version_available() -{ - new_version_available_dispatcher_.emit(); -} - /** * \brief set the detailed info panel on the right * \param[in] bottle - Wine Bottle item object @@ -866,16 +843,15 @@ void MainWindow::check_version_update(bool show_equal_or_error) else { // Start the check version thread - thread_check_version_ = new std::thread([this, show_equal_or_error] { check_version(this, show_equal_or_error); }); + thread_check_version_ = new std::thread([this, show_equal_or_error] { check_version(show_equal_or_error); }); } } /** * \brief Check WineGUI version (runs in thread) - * \param[in] mainWindow Main Window pointer, in order to signal back events * \param[in] show_equal_or_error Also show message when the versions matches or an error occurs. */ -void MainWindow::check_version(MainWindow* mainWindow, bool show_equal_or_error) +void MainWindow::check_version(bool show_equal_or_error) { string version = Helper::open_file_from_uri("https://winegui.melroy.org/latest_release.txt"); // Remove new lines @@ -889,7 +865,8 @@ void MainWindow::check_version(MainWindow* mainWindow, bool show_equal_or_error) std::lock_guard lock(new_version_mutex_); new_version_ = version; } - mainWindow->signal_new_version_available(); // Will eventually show a dialog + this->new_version_available_dispatcher_.emit(); // Will eventually show a dialog + return; } else { @@ -900,7 +877,8 @@ void MainWindow::check_version(MainWindow* mainWindow, bool show_equal_or_error) std::lock_guard lock(info_message_mutex_); info_message_ = "WineGUI release is up-to-date. Well done!"; } - mainWindow->signal_info_message_check_version(); + this->info_message_check_version_dispatcher_.emit(); + return; } } } @@ -912,9 +890,11 @@ void MainWindow::check_version(MainWindow* mainWindow, bool show_equal_or_error) std::lock_guard lock(error_message_mutex_); error_message_ = "We could not determine the latest WineGUI version. Try again later."; } - mainWindow->signal_error_message_check_version(); + this->error_message_check_version_dispatcher_.emit(); + return; } } + this->check_version_finished_dispatcher_.emit(); // Clean-up the thread pointer } /** From e108bc32cc197f86e3fb4df40bad0cb50da6b520 Mon Sep 17 00:00:00 2001 From: Melroy van den Berg Date: Tue, 21 May 2024 20:34:51 +0200 Subject: [PATCH 4/4] Apply 1 suggestion(s) to 1 file(s) --- src/main_window.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main_window.cc b/src/main_window.cc index fa86f3d..d36985b 100644 --- a/src/main_window.cc +++ b/src/main_window.cc @@ -837,7 +837,7 @@ void MainWindow::check_version_update(bool show_equal_or_error) { if (show_equal_or_error) { - show_error_message("WineGUI version check already running"); + show_error_message("WineGUI version check is stilling running. Please try again later."); } } else