Skip to content

Commit d1f44a5

Browse files
lijy91lijy91
committed
Add cleanup for GTK signal handlers and tray icon timeouts
Signal handler IDs for menu and menu item events are now tracked and properly disconnected in destructors to prevent accessing freed memory. Tray icon implementation now tracks GLib timeout source IDs for icon cleanup and cancels any pending timeouts in its destructor. These changes improve resource management and prevent potential crashes or memory issues.
1 parent 3c4a88f commit d1f44a5

File tree

2 files changed

+80
-14
lines changed

2 files changed

+80
-14
lines changed

‎src/platform/linux/menu_linux.cpp‎

Lines changed: 63 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,9 @@ class MenuItem::Impl {
8888
type_(type),
8989
state_(MenuItemState::Unchecked),
9090
radio_group_(-1),
91-
accelerator_("", KeyboardAccelerator::None) {}
91+
accelerator_("", KeyboardAccelerator::None),
92+
activate_handler_id_(0),
93+
toggled_handler_id_(0) {}
9294

9395
void ApplyRadioGroup() {
9496
if (!gtk_menu_item_ || type_ != MenuItemType::Radio || radio_group_ < 0) {
@@ -123,6 +125,10 @@ class MenuItem::Impl {
123125
int radio_group_;
124126
KeyboardAccelerator accelerator_;
125127
std::shared_ptr<Menu> submenu_;
128+
129+
// Signal handler IDs for cleanup
130+
gulong activate_handler_id_;
131+
gulong toggled_handler_id_;
126132

127133
// Shared map from logical group id to GTK group list
128134
static std::unordered_map<int, GSList*> s_group_map_;
@@ -165,9 +171,11 @@ MenuItem::MenuItem(const std::string& label, MenuItemType type) {
165171
// Connect signals for click/toggle events (except separators)
166172
if (gtk_item && type != MenuItemType::Separator) {
167173
if (type == MenuItemType::Checkbox || type == MenuItemType::Radio) {
168-
g_signal_connect(G_OBJECT(gtk_item), "toggled", G_CALLBACK(OnGtkCheckMenuItemToggled), this);
174+
pimpl_->toggled_handler_id_ = g_signal_connect(G_OBJECT(gtk_item), "toggled",
175+
G_CALLBACK(OnGtkCheckMenuItemToggled), this);
169176
} else {
170-
g_signal_connect(G_OBJECT(gtk_item), "activate", G_CALLBACK(OnGtkMenuItemActivate), this);
177+
pimpl_->activate_handler_id_ = g_signal_connect(G_OBJECT(gtk_item), "activate",
178+
G_CALLBACK(OnGtkMenuItemActivate), this);
171179
}
172180
}
173181
}
@@ -186,16 +194,39 @@ MenuItem::MenuItem(void* menu_item) {
186194

187195
if (pimpl_->gtk_menu_item_) {
188196
if (GTK_IS_CHECK_MENU_ITEM(pimpl_->gtk_menu_item_)) {
189-
g_signal_connect(G_OBJECT(pimpl_->gtk_menu_item_), "toggled",
190-
G_CALLBACK(OnGtkCheckMenuItemToggled), this);
197+
pimpl_->toggled_handler_id_ = g_signal_connect(G_OBJECT(pimpl_->gtk_menu_item_), "toggled",
198+
G_CALLBACK(OnGtkCheckMenuItemToggled), this);
191199
} else {
192-
g_signal_connect(G_OBJECT(pimpl_->gtk_menu_item_), "activate",
193-
G_CALLBACK(OnGtkMenuItemActivate), this);
200+
pimpl_->activate_handler_id_ = g_signal_connect(G_OBJECT(pimpl_->gtk_menu_item_), "activate",
201+
G_CALLBACK(OnGtkMenuItemActivate), this);
194202
}
195203
}
196204
}
197205

198-
MenuItem::~MenuItem() {}
206+
MenuItem::~MenuItem() {
207+
// Disconnect signal handlers before destruction to prevent accessing freed memory
208+
if (pimpl_->gtk_menu_item_) {
209+
if (pimpl_->activate_handler_id_ > 0) {
210+
g_signal_handler_disconnect(G_OBJECT(pimpl_->gtk_menu_item_), pimpl_->activate_handler_id_);
211+
pimpl_->activate_handler_id_ = 0;
212+
}
213+
if (pimpl_->toggled_handler_id_ > 0) {
214+
g_signal_handler_disconnect(G_OBJECT(pimpl_->gtk_menu_item_), pimpl_->toggled_handler_id_);
215+
pimpl_->toggled_handler_id_ = 0;
216+
}
217+
218+
// Disconnect submenu map/unmap handlers if they exist
219+
if (pimpl_->submenu_ && pimpl_->submenu_->GetNativeObject()) {
220+
GtkWidget* submenu_widget = (GtkWidget*)pimpl_->submenu_->GetNativeObject();
221+
if (submenu_widget) {
222+
g_signal_handlers_disconnect_by_func(G_OBJECT(submenu_widget),
223+
(gpointer)OnGtkSubmenuMap, this);
224+
g_signal_handlers_disconnect_by_func(G_OBJECT(submenu_widget),
225+
(gpointer)OnGtkSubmenuUnmap, this);
226+
}
227+
}
228+
}
229+
}
199230

200231
MenuItemId MenuItem::GetId() const {
201232
return pimpl_->id_;
@@ -424,34 +455,53 @@ void* MenuItem::GetNativeObjectInternal() const {
424455
// Private implementation class for Menu
425456
class Menu::Impl {
426457
public:
427-
Impl(MenuId id, GtkWidget* menu) : id_(id), gtk_menu_(menu) {}
458+
Impl(MenuId id, GtkWidget* menu)
459+
: id_(id), gtk_menu_(menu), map_handler_id_(0), unmap_handler_id_(0) {}
428460

429461
MenuId id_;
430462
GtkWidget* gtk_menu_;
431463
std::vector<std::shared_ptr<MenuItem>> items_;
464+
465+
// Signal handler IDs for cleanup
466+
gulong map_handler_id_;
467+
gulong unmap_handler_id_;
432468
};
433469

434470
Menu::Menu() {
435471
MenuId id = IdAllocator::Allocate<Menu>();
436472
pimpl_ = std::unique_ptr<Impl>(new Impl(id, gtk_menu_new()));
437473
// Connect menu map/unmap to emit open/close events when actually visible
438474
if (pimpl_->gtk_menu_) {
439-
g_signal_connect(G_OBJECT(pimpl_->gtk_menu_), "map", G_CALLBACK(OnGtkMenuMap), this);
440-
g_signal_connect(G_OBJECT(pimpl_->gtk_menu_), "unmap", G_CALLBACK(OnGtkMenuUnmap), this);
475+
pimpl_->map_handler_id_ = g_signal_connect(G_OBJECT(pimpl_->gtk_menu_), "map",
476+
G_CALLBACK(OnGtkMenuMap), this);
477+
pimpl_->unmap_handler_id_ = g_signal_connect(G_OBJECT(pimpl_->gtk_menu_), "unmap",
478+
G_CALLBACK(OnGtkMenuUnmap), this);
441479
}
442480
}
443481

444482
Menu::Menu(void* menu) {
445483
MenuId id = IdAllocator::Allocate<Menu>();
446484
pimpl_ = std::unique_ptr<Impl>(new Impl(id, (GtkWidget*)menu));
447485
if (pimpl_->gtk_menu_) {
448-
g_signal_connect(G_OBJECT(pimpl_->gtk_menu_), "map", G_CALLBACK(OnGtkMenuMap), this);
449-
g_signal_connect(G_OBJECT(pimpl_->gtk_menu_), "unmap", G_CALLBACK(OnGtkMenuUnmap), this);
486+
pimpl_->map_handler_id_ = g_signal_connect(G_OBJECT(pimpl_->gtk_menu_), "map",
487+
G_CALLBACK(OnGtkMenuMap), this);
488+
pimpl_->unmap_handler_id_ = g_signal_connect(G_OBJECT(pimpl_->gtk_menu_), "unmap",
489+
G_CALLBACK(OnGtkMenuUnmap), this);
450490
}
451491
}
452492

453493
Menu::~Menu() {
494+
// Disconnect signal handlers before unreferencing to prevent accessing freed memory
454495
if (pimpl_->gtk_menu_) {
496+
if (pimpl_->map_handler_id_ > 0) {
497+
g_signal_handler_disconnect(G_OBJECT(pimpl_->gtk_menu_), pimpl_->map_handler_id_);
498+
pimpl_->map_handler_id_ = 0;
499+
}
500+
if (pimpl_->unmap_handler_id_ > 0) {
501+
g_signal_handler_disconnect(G_OBJECT(pimpl_->gtk_menu_), pimpl_->unmap_handler_id_);
502+
pimpl_->unmap_handler_id_ = 0;
503+
}
504+
455505
g_object_unref(pimpl_->gtk_menu_);
456506
}
457507
}

‎src/platform/linux/tray_icon_linux.cpp‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,12 +37,23 @@ class TrayIcon::Impl {
3737
id_ = IdAllocator::Allocate<TrayIcon>();
3838
}
3939

40+
~Impl() {
41+
// Cancel any pending cleanup timeouts
42+
for (guint source_id : pending_cleanup_sources_) {
43+
if (source_id > 0) {
44+
g_source_remove(source_id);
45+
}
46+
}
47+
pending_cleanup_sources_.clear();
48+
}
49+
4050
AppIndicator* app_indicator_;
4151
std::shared_ptr<Menu> context_menu_; // Store menu shared_ptr to keep it alive
4252
std::optional<std::string> title_;
4353
std::optional<std::string> tooltip_;
4454
bool visible_;
4555
TrayIconId id_;
56+
std::vector<guint> pending_cleanup_sources_; // Track GLib timeout source IDs
4657
};
4758

4859
TrayIcon::TrayIcon() : pimpl_(std::make_unique<Impl>(nullptr)) {
@@ -131,14 +142,19 @@ void TrayIcon::SetIcon(std::shared_ptr<Image> image) {
131142
if (success) {
132143
// Set the icon and schedule cleanup
133144
app_indicator_set_icon_full(pimpl_->app_indicator_, png_path.c_str(), "");
134-
g_timeout_add(
145+
146+
// Track the cleanup timeout source ID so we can cancel it if needed
147+
guint source_id = g_timeout_add(
135148
5000,
136149
[](gpointer data) -> gboolean {
137150
unlink(static_cast<char*>(data));
138151
g_free(data);
139152
return FALSE; // Don't repeat
140153
},
141154
g_strdup(png_path.c_str()));
155+
156+
// Store source ID for cleanup in destructor
157+
pimpl_->pending_cleanup_sources_.push_back(source_id);
142158
} else {
143159
// Fallback to default icon
144160
app_indicator_set_icon_full(pimpl_->app_indicator_, "application-default-icon", "Tray Icon");

0 commit comments

Comments
 (0)