Refactored InstanceManager (now WindowInstanceManager) to manage all window instances, keep them alive and have the ability to destroy them; this avoids any manual "delete" calls, and fixes a (newly introduced) crash on exit.

src/applib/app_builder_widget.h

@@ -15,11 +15,12 @@ Copyright:
 #include "local_glibmm.h"
 #include <string>
 #include <gtkmm.h>
+#include <memory>
 
 #include "hz/debug.h"
-#include "hz/instance_manager.h"
 #include "hz/data_file.h"
 
+#include "window_instance_manager.h"
 #include "gui_utils.h"  // gui_show_error_dialog
 
 
@@ -52,11 +53,11 @@ Copyright:
 /// management and other benefits.
 /// If \c MultiInstance is false, create() will return the same instance each time.
 template<class Child, bool MultiInstance, class WidgetType = Gtk::Window>
-class AppBuilderWidget : public WidgetType, public hz::InstanceManager<Child, MultiInstance> {
+class AppBuilderWidget : public WidgetType, public WindowInstanceManager<Child, MultiInstance> {
 	public:
 
 		friend class Gtk::Builder;  // allow construction via GtkBuilder
-		// friend class hz::InstanceManager<Child, MultiInstance>;  // allow construction through instance class
+		// friend class WindowInstanceManager<Child, MultiInstance>;  // allow construction through instance class
 
 
 		/// Disallow
@@ -80,7 +81,7 @@ class AppBuilderWidget : public WidgetType, public hz::InstanceManager<Child, Mu
 		/// A glade file in "ui" data domain is loaded with Child::ui_name filename base and is available as
 		/// `get_ui()` in child object.
 		/// \return nullptr if widget could not be loaded.
-		static Child* create();
+		static std::shared_ptr<Child> create();
 
 
 		/// Get UI resource
@@ -127,10 +128,10 @@ class AppBuilderWidget : public WidgetType, public hz::InstanceManager<Child, Mu
 
 
 template<class Child, bool MultiInstance, class WidgetType>
-Child* AppBuilderWidget<Child, MultiInstance, WidgetType>::create()
+std::shared_ptr<Child> AppBuilderWidget<Child, MultiInstance, WidgetType>::create()
 {
 	if constexpr(!MultiInstance) {  // for single-instance objects
-		if (auto* inst = hz::InstanceManager<Child, MultiInstance>::instance()) {
+		if (auto inst = WindowInstanceManager<Child, MultiInstance>::instance()) {
 			return inst;
 		}
 	}
@@ -141,19 +142,16 @@ Child* AppBuilderWidget<Child, MultiInstance, WidgetType>::create()
 	try {
 		auto ui = Gtk::Builder::create_from_file(ui_path.u8string());  // may throw
 
-		Child* o = nullptr;
-		ui->get_widget_derived({Child::ui_name.data(), Child::ui_name.size()}, o);  // Calls Child's constructor
-
-		if (!o) {
+		Child* raw_obj = nullptr;
+		ui->get_widget_derived({Child::ui_name.data(), Child::ui_name.size()}, raw_obj);  // Calls Child's constructor
+		if (!raw_obj) {
 			debug_out_fatal("app", "Fatal error: Cannot get root widget from UI-resource-created hierarchy.\n");
 			gui_show_error_dialog(_("Fatal error: Cannot get root widget from UI-resource-created hierarchy."));
 			return nullptr;
 		}
 
-		if constexpr(!MultiInstance) {
-			hz::InstanceManager<Child, MultiInstance>::set_single_instance(o);  // for single-instance objects
-		}
-		return o;
+		// Store the instance so it does not get destroyed
+		return WindowInstanceManager<Child, MultiInstance>::store_instance(raw_obj);
 	}
 	catch (Glib::Exception& ex) {
 		error_msg = ex.what();

src/applib/window_instance_manager.h

@@ -0,0 +1,180 @@
+/******************************************************************************
+License: GNU General Public License v3.0 only
+Copyright:
+	(C) 2008 - 2021 Alexander Shaduri <ashaduri@gmail.com>
+******************************************************************************/
+/// \file
+/// \author Alexander Shaduri
+/// \ingroup applib
+/// \weakgroup applib
+/// @{
+
+#ifndef WINDOW_INSTANCE_MANAGER_H
+#define WINDOW_INSTANCE_MANAGER_H
+
+#include <memory>
+#include <unordered_set>
+#include "local_glibmm.h"
+#include <gtkmm.h>
+
+
+
+class WindowInstanceManagerStorage {
+	public:
+
+		/// Store an instance and keep it alive.
+		/// Return a newly stored shared pointer to the instance.
+		static std::shared_ptr<Gtk::Window> store_instance(Gtk::Window* obj)
+		{
+			std::shared_ptr<Gtk::Window> obj_sptr(obj);
+			instances_.insert(obj_sptr);
+			return obj_sptr;
+		}
+
+
+		/// Destroy a previously stored instance
+		static void destroy_instance(Gtk::Window* window)
+		{
+			auto found = std::find_if(instances_.begin(), instances_.end(), [window](const std::shared_ptr<Gtk::Window>& elem) { return elem.get() == window; });
+			if (found != instances_.end()) {
+				instances_.erase(found);
+			}
+		}
+
+
+		/// Destroy all stored instances
+		static void destroy_all_instances()
+		{
+			instances_.clear();
+		}
+
+
+	private:
+		/// All instances of created objects, kept alive by shared_ptr
+		static inline std::unordered_set<std::shared_ptr<Gtk::Window>> instances_;
+
+};
+
+
+
+/// Inherit this class template to have a single- or multi-instance objects, e.g. windows.
+/// This is a multi-instance implementation.
+template<class Child, bool MultiInstance>
+class WindowInstanceManager {
+	protected:
+
+		/// Can't construct / delete this directly! use create() and destroy()
+		WindowInstanceManager() = default;
+
+
+	public:
+
+		/// Deleted
+		WindowInstanceManager(const WindowInstanceManager& other) = delete;
+
+		/// Deleted
+		WindowInstanceManager(const WindowInstanceManager&& other) = delete;
+
+		/// Deleted
+		WindowInstanceManager& operator=(const WindowInstanceManager&) = delete;
+
+		/// Deleted
+		WindowInstanceManager& operator=(const WindowInstanceManager&&) = delete;
+
+		/// Default, must be polymorphic for casts to succeed
+		virtual ~WindowInstanceManager() = default;
+
+
+		/// The default multi-instance implementation doesn't support `instance()`
+		static Child* instance() = delete;
+
+
+		/// Destroy a previously stored instance
+		void destroy_instance()
+		{
+			WindowInstanceManagerStorage::destroy_instance(dynamic_cast<Gtk::Window*>(this));  // side-cast
+		}
+
+
+	protected:
+
+		/// Store an instance and keep it alive.
+		/// Return a newly stored shared pointer to the instance.
+		static std::shared_ptr<Child> store_instance(Child* obj)
+		{
+			return std::dynamic_pointer_cast<Child>(WindowInstanceManagerStorage::store_instance(obj));
+		}
+
+};
+
+
+
+
+/// Single-instance specialization. This deletes the instance on program exit.
+template<class Child>
+class WindowInstanceManager<Child, false> {
+	protected:
+
+		/// Can't construct / delete this directly! use create() and destroy()
+		WindowInstanceManager() = default;
+
+
+	public:
+
+		/// Deleted
+		WindowInstanceManager(const WindowInstanceManager& other) = delete;
+
+		/// Deleted
+		WindowInstanceManager(const WindowInstanceManager&& other) = delete;
+
+		/// Deleted
+		WindowInstanceManager& operator=(const WindowInstanceManager&) = delete;
+
+		/// Deleted
+		WindowInstanceManager& operator=(const WindowInstanceManager&&) = delete;
+
+		/// Default, must be polymorphic for casts to succeed
+		virtual ~WindowInstanceManager() = default;
+
+
+		/// Return a single existing instance of this template instantiation.
+		/// \return nullptr if no instances were created yet.
+		static std::shared_ptr<Child> instance()
+		{
+			return instance_.lock();
+		}
+
+
+		/// Destroy a previously stored instance
+		void destroy_instance()
+		{
+			WindowInstanceManagerStorage::destroy_instance(dynamic_cast<Gtk::Window*>(this));  // side-cast
+		}
+
+
+	protected:
+
+		/// Store an instance and keep it alive.
+		/// Return a newly stored shared pointer to the instance.
+		static std::shared_ptr<Child> store_instance(Child* obj)
+		{
+			auto inst = std::dynamic_pointer_cast<Child>(WindowInstanceManagerStorage::store_instance(obj));
+			instance_ = inst;
+			return inst;
+		}
+
+
+	private:
+
+		static inline std::weak_ptr<Child> instance_;  ///< Single instance pointer
+
+};
+
+
+
+
+
+
+#endif
+
+/// @}

src/gsc_about_dialog.cpp

@@ -78,7 +78,7 @@ void GscAboutDialog::on_response(int response_id)
 	if (response_id == Gtk::RESPONSE_NONE || response_id == Gtk::RESPONSE_DELETE_EVENT
 			|| response_id == Gtk::RESPONSE_CANCEL || response_id == Gtk::RESPONSE_CLOSE) {
 		debug_out_info("app", DBG_FUNC_MSG << "Closing the dialog.\n");
-		delete this;  // close the window and delete the object
+		destroy_instance();  // close the window and delete the object
 	}
 }
 

src/gsc_add_device_window.cpp

@@ -138,7 +138,7 @@ bool GscAddDeviceWindow::on_delete_event([[maybe_unused]] GdkEventAny* e)
 
 void GscAddDeviceWindow::on_window_cancel_button_clicked()
 {
-	delete this;
+	this->destroy_instance();
 }
 
 
@@ -159,7 +159,7 @@ void GscAddDeviceWindow::on_window_ok_button_clicked()
 		main_window_->add_device(dev, type, params);
 	}
 
-	delete this;
+	destroy_instance();
 }
 
 

src/gsc_add_device_window.h

@@ -42,7 +42,7 @@ class GscAddDeviceWindow : public AppBuilderWidget<GscAddDeviceWindow, true> {
 	protected:
 
 
-		// ---------- overriden virtual methods
+		// ---------- overridden virtual methods
 
 		/// Destroy this object on delete event (by default it calls hide()).
 		/// Reimplemented from Gtk::Window.

src/gsc_executor_error_dialog.cpp

@@ -75,7 +75,7 @@ void gsc_executor_error_dialog_show(const std::string& message, const std::strin
 
 	if (response == Gtk::RESPONSE_HELP) {
 		// this one will only hide on close.
-		GscExecutorLogWindow* win = GscExecutorLogWindow::create();  // probably already created
+		auto win = GscExecutorLogWindow::create();  // probably already created
 		// win->set_transient_for(*this);  // don't do this - it will make it always-on-top of this.
 		win->show_last();  // show the window and select last entry
 	}
@@ -91,7 +91,7 @@ void gsc_no_info_dialog_show(const std::string& message, const std::string& sec_
 			parent, sec_msg_markup, !output.empty());
 
 	if (response == Gtk::RESPONSE_HELP) {
-		GscTextWindow<SmartctlOutputInstance>* win = GscTextWindow<SmartctlOutputInstance>::create();
+		auto win = GscTextWindow<SmartctlOutputInstance>::create();
 		win->set_text_from_command(output_window_title, output);
 
 		if (!default_save_filename.empty())

src/gsc_info_window.cpp

@@ -714,7 +714,7 @@ void GscInfoWindow::on_refresh_info_button_clicked()
 
 void GscInfoWindow::on_view_output_button_clicked()
 {
-	GscTextWindow<SmartctlOutputInstance>* win = GscTextWindow<SmartctlOutputInstance>::create();
+	auto win = GscTextWindow<SmartctlOutputInstance>::create();
 	// make save visible and enable monospace font
 
 	std::string output = this->drive->get_full_output();
@@ -838,7 +838,7 @@ void GscInfoWindow::on_close_window_button_clicked()
 	if (drive && drive->get_test_is_active()) {  // disallow close if test is active.
 		gui_show_warn_dialog(_("Please wait until all tests are finished."), this);
 	} else {
-		delete this;  // deletes this object and nullifies instance
+		destroy_instance();  // deletes this object and nullifies instance
 	}
 }
 

src/gsc_init.cpp

@@ -41,6 +41,7 @@ Copyright:
 #include "hz/string_num.h"
 #include "build_config.h"  // VERSION, *PACKAGE*, ...
 
+#include "applib/window_instance_manager.h"
 #include "gsc_main_window.h"
 #include "gsc_executor_log_window.h"
 #include "gsc_settings.h"
@@ -521,35 +522,33 @@ bool app_init_and_loop(int& argc, char**& argv)
 
 	// Create executor log window, but don't show it.
 	// It will track all command executor outputs.
+	// The window is destroyed by the instance manager.
 	GscExecutorLogWindow::create();
 
 
-	// Open the main window
-	GscMainWindow* win = GscMainWindow::create();
-	if (!win) {
-		debug_out_fatal("app", "Cannot create the main window. Exiting.\n");
-		return false;  // cannot create main window
+	// Open the main window.
+	// The window is destroyed by the instance manager.
+	{
+		auto main_window = GscMainWindow::create();
+		if (!main_window) {
+			debug_out_fatal("app", "Cannot create the main window. Exiting.\n");
+			return false;  // cannot create main window
+		}
+
+		// first-boot message
+		// app_show_first_boot_message(win);
+
+		// The Main Loop
+		debug_out_info("app", "Entering main loop.\n");
+		Gtk::Main::run();
+		debug_out_info("app", "Main loop exited.\n");
 	}
 
-
-	// first-boot message
-	// app_show_first_boot_message(win);
-
-
-	// The Main Loop (tm)
-	debug_out_info("app", "Entering main loop.\n");
-	Gtk::Main::run();
-	debug_out_info("app", "Main loop exited.\n");
-
-	// close the main window and delete its object
-	delete GscMainWindow::instance();
-
-	delete GscExecutorLogWindow::instance();
-
+	// Destroy all windows manually, to avoid surprises
+	WindowInstanceManagerStorage::destroy_all_instances();
 
 	// std::cerr << app_get_debug_buffer_str();  // this will output everything that went through libdebug.
 
-
 	return true;
 }
 

src/gsc_main_window.cpp

@@ -155,7 +155,8 @@ GscMainWindow::~GscMainWindow()
 	// This is needed because for some reason, if any icon is selected,
 	// on_iconview_selection_changed() is called even after the window is deleted,
 	// causing crash on exit.
-	iconview_->clear_all();
+	// iconview_->clear_all();
+	delete iconview_;
 }
 
 
@@ -535,7 +536,7 @@ void GscMainWindow::on_action_activated(GscMainWindow::action_t action_type)
 
 		case action_perform_tests:
 			if (iconview_) {
-				GscInfoWindow* win = this->show_device_info_window(iconview_->get_selected_drive());
+				auto win = this->show_device_info_window(iconview_->get_selected_drive());
 				if (win)  // won't be created if test is already running
 					win->show_tests();
 			}
@@ -573,7 +574,7 @@ void GscMainWindow::on_action_activated(GscMainWindow::action_t action_type)
 		case action_executor_log:
 		{
 			// this one will only hide on close.
-			GscExecutorLogWindow* win = GscExecutorLogWindow::create();  // probably already created
+			auto win = GscExecutorLogWindow::create();  // probably already created
 			// win->set_transient_for(*this);  // don't do this - it will make it always-on-top of this.
 			win->show_last();  // show the window and select last entry
 			break;
@@ -587,7 +588,7 @@ void GscMainWindow::on_action_activated(GscMainWindow::action_t action_type)
 
 		case action_preferences:
 		{
-			GscPreferencesWindow* win = GscPreferencesWindow::create();  // destroyed on close
+			auto win = GscPreferencesWindow::create();  // destroyed on close
 			win->set_transient_for(*this);  // for "destroy with parent", always-on-top
 			win->set_main_window(this);
 			win->set_modal(true);
@@ -609,7 +610,7 @@ void GscMainWindow::on_action_activated(GscMainWindow::action_t action_type)
 
 		case action_about:
 		{
-			GscAboutDialog* dialog = GscAboutDialog::create();  // destroyed on close
+			auto dialog = GscAboutDialog::create();  // destroyed on close
 			dialog->set_transient_for(*this);  // for "destroy with parent"
 			dialog->show();
 			break;
@@ -1184,7 +1185,7 @@ bool GscMainWindow::testing_active() const
 
 
 
-GscInfoWindow* GscMainWindow::show_device_info_window(const StorageDevicePtr& drive)
+std::shared_ptr<GscInfoWindow> GscMainWindow::show_device_info_window(const StorageDevicePtr& drive)
 {
 	// if a test is being run on it, disallow.
 	if (drive->get_test_is_active()) {
@@ -1245,7 +1246,7 @@ GscInfoWindow* GscMainWindow::show_device_info_window(const StorageDevicePtr& dr
 	}
 
 
-	GscInfoWindow* win = GscInfoWindow::create();  // self-destroyed
+	auto win = GscInfoWindow::create();  // self-destroyed
 
 	win->set_drive(drive);
 	win->fill_ui_with_info(false);  // already scanned. "refresh" will scan it again in the info window.
@@ -1272,7 +1273,7 @@ void GscMainWindow::show_prefs_updated_message()
 
 void GscMainWindow::show_add_device_chooser()
 {
-	GscAddDeviceWindow* window = GscAddDeviceWindow::create();
+	auto window = GscAddDeviceWindow::create();
 	window->set_main_window(this);
 	window->set_transient_for(*this);
 	window->show();

src/gsc_main_window.h

@@ -65,7 +65,7 @@ class GscMainWindow : public AppBuilderWidget<GscMainWindow, false> {
 
 
 		/// Show the info window for the drive
-		GscInfoWindow* show_device_info_window(const StorageDevicePtr& drive);
+		std::shared_ptr<GscInfoWindow> show_device_info_window(const StorageDevicePtr& drive);
 
 		/// Show "Preferences updated, please rescan" message
 		void show_prefs_updated_message();

src/gsc_main_window_iconview.h

@@ -169,6 +169,9 @@ class GscMainWindowIconView : public Gtk::IconView {
 		// Overridden from Gtk::Widget
 		bool on_draw(const Cairo::RefPtr<Cairo::Context>& cr) override
 		{
+			if (in_destruction()) {
+				return true;
+			}
 			if (empty_view_message != Message::none && this->num_icons == 0) {  // no icons
 				Glib::RefPtr<Pango::Layout> layout = this->create_pango_layout("");
 				layout->set_alignment(Pango::ALIGN_CENTER);

src/gsc_preferences_window.cpp

@@ -458,7 +458,7 @@ bool GscPreferencesWindow::on_delete_event([[maybe_unused]] GdkEventAny* e)
 
 void GscPreferencesWindow::on_window_cancel_button_clicked()
 {
-	delete this;
+	destroy_instance();
 }
 
 
@@ -491,7 +491,7 @@ void GscPreferencesWindow::on_window_ok_button_clicked()
 		main_window_->show_prefs_updated_message();
 	}
 
-	delete this;
+	destroy_instance();
 }
 
 
@@ -506,7 +506,7 @@ void GscPreferencesWindow::on_window_reset_all_button_clicked()
 		rconfig::clear_config();
 		import_config();
 		// close the window, because the user might get the impression that "Cancel" will revert.
-		delete this;
+		destroy_instance();
 	}
 }
 

src/gsc_text_window.h

@@ -252,7 +252,7 @@ class GscTextWindow : public AppBuilderWidget<GscTextWindow<InstanceSwitch>, Ins
 		/// Button click callback
 		void on_close_window_button_clicked()
 		{
-			delete this;
+			this->destroy_instance();
 		}
 
 

src/hz/instance_manager.h

@@ -1,119 +0,0 @@
-/******************************************************************************
-License: Zlib
-Copyright:
-	(C) 2008 - 2021 Alexander Shaduri <ashaduri@gmail.com>
-******************************************************************************/
-/// \file
-/// \author Alexander Shaduri
-/// \ingroup hz
-/// \weakgroup hz
-/// @{
-
-#ifndef HZ_INSTANCE_MANAGER_H
-#define HZ_INSTANCE_MANAGER_H
-
-#include <memory>
-
-
-
-namespace hz {
-
-
-/// Inherit this class template to have a single- or multi-instance objects, e.g. windows.
-/// This is a multi-instance implementation.
-template<class Child, bool MultiInstance>
-class InstanceManager {
-	protected:
-
-		/// Can't construct / delete this directly! use create() and destroy()
-		InstanceManager() = default;
-
-
-	public:
-
-		/// Deleted
-		InstanceManager(const InstanceManager& other) = delete;
-
-		/// Deleted
-		InstanceManager(const InstanceManager&& other) = delete;
-
-		/// Deleted
-		InstanceManager& operator=(const InstanceManager&) = delete;
-
-		/// Deleted
-		InstanceManager& operator=(const InstanceManager&&) = delete;
-
-		/// Default
-		~InstanceManager() = default;
-
-
-		/// The default multi-instance implementation doesn't support `instance()`
-		static Child* instance() = delete;
-
-};
-
-
-
-
-/// Single-instance specialization
-template<class Child>
-class InstanceManager<Child, false> {
-	protected:
-
-		/// Can't construct / delete this directly! use create() and destroy()
-		InstanceManager() = default;
-
-
-	public:
-
-		/// Deleted
-		InstanceManager(const InstanceManager& other) = delete;
-
-		/// Deleted
-		InstanceManager(const InstanceManager&& other) = delete;
-
-		/// Deleted
-		InstanceManager& operator=(const InstanceManager&) = delete;
-
-		/// Deleted
-		InstanceManager& operator=(const InstanceManager&&) = delete;
-
-		/// Default
-		~InstanceManager() = default;
-
-
-		/// Return a single existing instance of this template instantiation.
-		/// \return nullptr if no instances were created yet.
-		static Child* instance()
-		{
-			return instance_.get();
-		}
-
-
-	protected:
-
-		/// Set the instance.
-		static void set_single_instance(Child* instance)
-		{
-			instance_.reset(instance);
-		}
-
-
-	private:
-
-		static inline std::unique_ptr<Child> instance_;  ///< Single instance pointer
-
-};
-
-
-
-
-
-}  // ns
-
-
-
-
-#endif
-
-/// @}