Disconnect some more signals on widgets destruction (#4379)

This is applicable to any signals where emitter's lifetime isn't
controlled by the receiver.

There're still about 7 `Glib::signal_idle()` connections which aren't
tracked but I see the possibility of it leading to major issues as quite
low.
This commit is contained in:
Mike Gelfand
2022-12-16 09:40:28 -08:00
committed by GitHub
parent e6d75a4b77
commit 0824c2da6d
5 changed files with 38 additions and 36 deletions

View File

@@ -964,11 +964,9 @@ void Application::Impl::on_app_exit()
is_closing_ = true; is_closing_ = true;
/* stop the update timer */
timer_.disconnect();
/* stop the refresh-actions timer */
refresh_actions_tag_.disconnect(); refresh_actions_tag_.disconnect();
update_model_soon_tag_.disconnect();
timer_.disconnect();
#if !GTKMM_CHECK_VERSION(4, 0, 0) #if !GTKMM_CHECK_VERSION(4, 0, 0)
wind_->remove(); wind_->remove();

View File

@@ -168,7 +168,6 @@ private:
Glib::Quark const URL_ENTRY_KEY = Glib::Quark("tr-url-entry-key"); Glib::Quark const URL_ENTRY_KEY = Glib::Quark("tr-url-entry-key");
static guint last_page_; static guint last_page_;
sigc::connection last_page_tag_;
}; };
guint DetailsDialog::Impl::last_page_ = 0; guint DetailsDialog::Impl::last_page_ = 0;
@@ -2502,7 +2501,6 @@ void DetailsDialog::Impl::on_details_window_size_allocated()
DetailsDialog::Impl::~Impl() DetailsDialog::Impl::~Impl()
{ {
periodic_refresh_tag_.disconnect(); periodic_refresh_tag_.disconnect();
last_page_tag_.disconnect();
} }
std::unique_ptr<DetailsDialog> DetailsDialog::create(Gtk::Window& parent, Glib::RefPtr<Session> const& core) std::unique_ptr<DetailsDialog> DetailsDialog::create(Gtk::Window& parent, Glib::RefPtr<Session> const& core)
@@ -2589,7 +2587,7 @@ DetailsDialog::Impl::Impl(DetailsDialog& dialog, Glib::RefPtr<Gtk::Builder> cons
auto* const n = gtr_get_widget<Gtk::Notebook>(builder, "dialog_pages"); auto* const n = gtr_get_widget<Gtk::Notebook>(builder, "dialog_pages");
n->set_current_page(last_page_); n->set_current_page(last_page_);
last_page_tag_ = n->signal_switch_page().connect([](Widget*, guint page) { DetailsDialog::Impl::last_page_ = page; }); n->signal_switch_page().connect([](Gtk::Widget* /*page*/, guint page_number) { last_page_ = page_number; });
} }
void DetailsDialog::set_torrents(std::vector<tr_torrent_id_t> const& ids) void DetailsDialog::set_torrents(std::vector<tr_torrent_id_t> const& ids)

View File

@@ -7,6 +7,7 @@
#include <cstddef> #include <cstddef>
#include <list> #include <list>
#include <memory> #include <memory>
#include <queue>
#include <stack> #include <stack>
#include <string> #include <string>
#include <string_view> #include <string_view>
@@ -106,7 +107,8 @@ private:
bool onViewPathToggled(Gtk::TreeViewColumn* col, Gtk::TreeModel::Path const& path); bool onViewPathToggled(Gtk::TreeViewColumn* col, Gtk::TreeModel::Path const& path);
void onRowActivated(Gtk::TreeModel::Path const& path, Gtk::TreeViewColumn* col); void onRowActivated(Gtk::TreeModel::Path const& path, Gtk::TreeViewColumn* col);
void cell_edited_callback(Glib::ustring const& path_string, Glib::ustring const& newname); void cell_edited_callback(Glib::ustring const& path_string, Glib::ustring const& newname);
bool on_rename_done_idle(Glib::ustring const& path_string, Glib::ustring const& newname, int error); void on_rename_done(Glib::ustring const& path_string, Glib::ustring const& newname, int error);
void on_rename_done_idle(Glib::ustring const& path_string, Glib::ustring const& newname, int error);
private: private:
FileList& widget_; FileList& widget_;
@@ -117,20 +119,24 @@ private:
Glib::RefPtr<Gtk::TreeStore> store_; Glib::RefPtr<Gtk::TreeStore> store_;
tr_torrent_id_t torrent_id_ = {}; tr_torrent_id_t torrent_id_ = {};
sigc::connection timeout_tag_; sigc::connection timeout_tag_;
std::queue<sigc::connection> rename_done_tags_;
}; };
void FileList::Impl::clearData() void FileList::Impl::clearData()
{ {
torrent_id_ = -1; torrent_id_ = -1;
if (timeout_tag_.connected()) timeout_tag_.disconnect();
{
timeout_tag_.disconnect();
}
} }
FileList::Impl::~Impl() FileList::Impl::~Impl()
{ {
while (!rename_done_tags_.empty())
{
rename_done_tags_.front().disconnect();
rename_done_tags_.pop();
}
clearData(); clearData();
} }
@@ -780,7 +786,18 @@ struct rename_data
gpointer impl = nullptr; gpointer impl = nullptr;
}; };
bool FileList::Impl::on_rename_done_idle(Glib::ustring const& path_string, Glib::ustring const& newname, int error) void FileList::Impl::on_rename_done(Glib::ustring const& path_string, Glib::ustring const& newname, int error)
{
rename_done_tags_.push(Glib::signal_idle().connect(
[this, path_string, newname, error]()
{
rename_done_tags_.pop();
on_rename_done_idle(path_string, newname, error);
return false;
}));
}
void FileList::Impl::on_rename_done_idle(Glib::ustring const& path_string, Glib::ustring const& newname, int error)
{ {
if (error == 0) if (error == 0)
{ {
@@ -817,8 +834,6 @@ bool FileList::Impl::on_rename_done_idle(Glib::ustring const& path_string, Glib:
w->signal_response().connect([w](int /*response*/) mutable { w.reset(); }); w->signal_response().connect([w](int /*response*/) mutable { w.reset(); });
w->show(); w->show();
} }
return false;
} }
void FileList::Impl::cell_edited_callback(Glib::ustring const& path_string, Glib::ustring const& newname) void FileList::Impl::cell_edited_callback(Glib::ustring const& path_string, Glib::ustring const& newname)
@@ -864,10 +879,8 @@ void FileList::Impl::cell_edited_callback(Glib::ustring const& path_string, Glib
static_cast<tr_torrent_rename_done_func>( static_cast<tr_torrent_rename_done_func>(
[](tr_torrent* /*tor*/, char const* /*oldpath*/, char const* /*newname*/, int error, gpointer data) [](tr_torrent* /*tor*/, char const* /*oldpath*/, char const* /*newname*/, int error, gpointer data)
{ {
Glib::signal_idle().connect( auto const data_grave = std::unique_ptr<struct rename_data>(static_cast<struct rename_data*>(data));
[rdata = std::shared_ptr<struct rename_data>(static_cast<struct rename_data*>(data)), error]() { static_cast<Impl*>(data_grave->impl)->on_rename_done(data_grave->path_string, data_grave->newname, error);
return static_cast<Impl*>(rdata->impl)->on_rename_done_idle(rdata->path_string, rdata->newname, error);
});
}), }),
rename_data.release()); rename_data.release());
} }

View File

@@ -398,10 +398,7 @@ void DownloadingPage::on_core_prefs_changed(tr_quark const key)
DownloadingPage::~DownloadingPage() DownloadingPage::~DownloadingPage()
{ {
if (core_prefs_tag_.connected()) core_prefs_tag_.disconnect();
{
core_prefs_tag_.disconnect();
}
} }
DownloadingPage::DownloadingPage( DownloadingPage::DownloadingPage(
@@ -535,10 +532,7 @@ void PrivacyPage::updateBlocklistText()
/* prefs dialog is being destroyed, so stop listening to blocklist updates */ /* prefs dialog is being destroyed, so stop listening to blocklist updates */
PrivacyPage::~PrivacyPage() PrivacyPage::~PrivacyPage()
{ {
if (updateBlocklistTag_.connected()) updateBlocklistTag_.disconnect();
{
updateBlocklistTag_.disconnect();
}
} }
/* user hit "close" in the blocklist-update dialog */ /* user hit "close" in the blocklist-update dialog */
@@ -901,15 +895,8 @@ void NetworkPage::onCorePrefsChanged(tr_quark const key)
NetworkPage::~NetworkPage() NetworkPage::~NetworkPage()
{ {
if (prefsTag_.connected()) prefsTag_.disconnect();
{ portTag_.disconnect();
prefsTag_.disconnect();
}
if (portTag_.connected())
{
portTag_.disconnect();
}
} }
void NetworkPage::onPortTested(bool isOpen) void NetworkPage::onPortTested(bool isOpen)

View File

@@ -90,6 +90,7 @@ class Session::Impl
{ {
public: public:
Impl(Session& core, tr_session* session); Impl(Session& core, tr_session* session);
~Impl();
tr_session* close(); tr_session* close();
@@ -849,6 +850,11 @@ Session::Impl::Impl(Session& core, tr_session* session)
this); this);
} }
Session::Impl::~Impl()
{
monitor_idle_tag_.disconnect();
}
tr_session* Session::close() tr_session* Session::close()
{ {
return impl_->close(); return impl_->close();