Skip to content

Commit 3bf3e25

Browse files
committed
Fix shared_ptr lifetime management for menu and tray icon callbacks
Ensure menu and menu item objects are kept alive via global storage. Switch tray icon callback data to shared_ptr for safe lambda capture. Clear tray icon callbacks on destruction to prevent dangling references. Protect macOS tray icon callbacks against exceptions.
1 parent 8b02b30 commit 3bf3e25

File tree

3 files changed

+68
-27
lines changed

3 files changed

+68
-27
lines changed

src/capi/menu_c.cpp

Lines changed: 34 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,14 @@
33
#include <cstring>
44
#include <map>
55
#include <memory>
6+
#include <unordered_map>
67

78
using namespace nativeapi;
89

10+
// Global object storage to manage shared_ptr lifetimes
11+
static std::unordered_map<void*, std::shared_ptr<MenuItem>> g_menu_items;
12+
static std::unordered_map<void*, std::shared_ptr<Menu>> g_menus;
13+
914
// Internal structures to manage C callbacks
1015
struct MenuItemCallbackData {
1116
native_menu_item_click_callback_t click_callback;
@@ -106,7 +111,9 @@ native_menu_item_t native_menu_item_create(const char* text, native_menu_item_ty
106111

107112
try {
108113
auto item = MenuItem::Create(text, convert_menu_item_type(type));
109-
return static_cast<native_menu_item_t>(item.get());
114+
void* handle = item.get();
115+
g_menu_items[handle] = item; // Store shared_ptr to keep object alive
116+
return static_cast<native_menu_item_t>(handle);
110117
} catch (...) {
111118
return nullptr;
112119
}
@@ -115,7 +122,9 @@ native_menu_item_t native_menu_item_create(const char* text, native_menu_item_ty
115122
native_menu_item_t native_menu_item_create_separator(void) {
116123
try {
117124
auto item = MenuItem::CreateSeparator();
118-
return static_cast<native_menu_item_t>(item.get());
125+
void* handle = item.get();
126+
g_menu_items[handle] = item; // Store shared_ptr to keep object alive
127+
return static_cast<native_menu_item_t>(handle);
119128
} catch (...) {
120129
return nullptr;
121130
}
@@ -130,8 +139,11 @@ void native_menu_item_destroy(native_menu_item_t item) {
130139
g_menu_item_callbacks.erase(it);
131140
}
132141

133-
// Note: The actual MenuItem object is managed by shared_ptr
134-
// This just removes our reference to it
142+
// Remove from global storage - this will release the shared_ptr
143+
auto item_it = g_menu_items.find(item);
144+
if (item_it != g_menu_items.end()) {
145+
g_menu_items.erase(item_it);
146+
}
135147
}
136148

137149
native_menu_item_id_t native_menu_item_get_id(native_menu_item_t item) {
@@ -495,7 +507,9 @@ bool native_menu_item_trigger(native_menu_item_t item) {
495507
native_menu_t native_menu_create(void) {
496508
try {
497509
auto menu = Menu::Create();
498-
return static_cast<native_menu_t>(menu.get());
510+
void* handle = menu.get();
511+
g_menus[handle] = menu; // Store shared_ptr to keep object alive
512+
return static_cast<native_menu_t>(handle);
499513
} catch (...) {
500514
return nullptr;
501515
}
@@ -510,8 +524,11 @@ void native_menu_destroy(native_menu_t menu) {
510524
g_menu_callbacks.erase(it);
511525
}
512526

513-
// Note: The actual Menu object is managed by shared_ptr
514-
// This just removes our reference to it
527+
// Remove from global storage - this will release the shared_ptr
528+
auto menu_it = g_menus.find(menu);
529+
if (menu_it != g_menus.end()) {
530+
g_menus.erase(menu_it);
531+
}
515532
}
516533

517534
native_menu_id_t native_menu_get_id(native_menu_t menu) {
@@ -530,8 +547,11 @@ void native_menu_add_item(native_menu_t menu, native_menu_item_t item) {
530547

531548
try {
532549
auto menu_ptr = static_cast<Menu*>(menu);
533-
auto item_ptr = std::shared_ptr<MenuItem>(static_cast<MenuItem*>(item));
534-
menu_ptr->AddItem(item_ptr);
550+
// Get the shared_ptr from global storage instead of creating a new one
551+
auto item_it = g_menu_items.find(item);
552+
if (item_it != g_menu_items.end()) {
553+
menu_ptr->AddItem(item_it->second);
554+
}
535555
} catch (...) {
536556
// Ignore exceptions
537557
}
@@ -542,8 +562,11 @@ void native_menu_insert_item(native_menu_t menu, native_menu_item_t item, size_t
542562

543563
try {
544564
auto menu_ptr = static_cast<Menu*>(menu);
545-
auto item_ptr = std::shared_ptr<MenuItem>(static_cast<MenuItem*>(item));
546-
menu_ptr->InsertItem(index, item_ptr);
565+
// Get the shared_ptr from global storage instead of creating a new one
566+
auto item_it = g_menu_items.find(item);
567+
if (item_it != g_menu_items.end()) {
568+
menu_ptr->InsertItem(index, item_it->second);
569+
}
547570
} catch (...) {
548571
// Ignore exceptions
549572
}

src/capi/tray_icon_c.cpp

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ struct TrayIconCallbackData {
1717
};
1818

1919
// Global map to store callback data
20-
static std::map<native_tray_icon_t, std::unique_ptr<TrayIconCallbackData>> g_tray_icon_callbacks;
20+
static std::map<native_tray_icon_t, std::shared_ptr<TrayIconCallbackData>> g_tray_icon_callbacks;
2121

2222
// TrayIcon C API Implementation
2323

@@ -224,16 +224,16 @@ void native_tray_icon_set_on_left_click(native_tray_icon_t tray_icon, native_tra
224224
// Get or create callback data
225225
auto it = g_tray_icon_callbacks.find(tray_icon);
226226
if (it == g_tray_icon_callbacks.end()) {
227-
g_tray_icon_callbacks[tray_icon] = std::make_unique<TrayIconCallbackData>();
227+
g_tray_icon_callbacks[tray_icon] = std::make_shared<TrayIconCallbackData>();
228228
}
229229

230-
auto& callback_data = g_tray_icon_callbacks[tray_icon];
230+
auto callback_data = g_tray_icon_callbacks[tray_icon];
231231
callback_data->left_click_callback = callback;
232232
callback_data->left_click_user_data = user_data;
233233

234234
if (callback) {
235-
tray_icon_ptr->SetOnLeftClick([tray_icon, callback_data = callback_data.get()]() {
236-
if (callback_data->left_click_callback) {
235+
tray_icon_ptr->SetOnLeftClick([callback_data]() {
236+
if (callback_data && callback_data->left_click_callback) {
237237
callback_data->left_click_callback(callback_data->left_click_user_data);
238238
}
239239
});
@@ -254,16 +254,16 @@ void native_tray_icon_set_on_right_click(native_tray_icon_t tray_icon, native_tr
254254
// Get or create callback data
255255
auto it = g_tray_icon_callbacks.find(tray_icon);
256256
if (it == g_tray_icon_callbacks.end()) {
257-
g_tray_icon_callbacks[tray_icon] = std::make_unique<TrayIconCallbackData>();
257+
g_tray_icon_callbacks[tray_icon] = std::make_shared<TrayIconCallbackData>();
258258
}
259259

260-
auto& callback_data = g_tray_icon_callbacks[tray_icon];
260+
auto callback_data = g_tray_icon_callbacks[tray_icon];
261261
callback_data->right_click_callback = callback;
262262
callback_data->right_click_user_data = user_data;
263263

264264
if (callback) {
265-
tray_icon_ptr->SetOnRightClick([tray_icon, callback_data = callback_data.get()]() {
266-
if (callback_data->right_click_callback) {
265+
tray_icon_ptr->SetOnRightClick([callback_data]() {
266+
if (callback_data && callback_data->right_click_callback) {
267267
callback_data->right_click_callback(callback_data->right_click_user_data);
268268
}
269269
});
@@ -284,16 +284,16 @@ void native_tray_icon_set_on_double_click(native_tray_icon_t tray_icon, native_t
284284
// Get or create callback data
285285
auto it = g_tray_icon_callbacks.find(tray_icon);
286286
if (it == g_tray_icon_callbacks.end()) {
287-
g_tray_icon_callbacks[tray_icon] = std::make_unique<TrayIconCallbackData>();
287+
g_tray_icon_callbacks[tray_icon] = std::make_shared<TrayIconCallbackData>();
288288
}
289289

290-
auto& callback_data = g_tray_icon_callbacks[tray_icon];
290+
auto callback_data = g_tray_icon_callbacks[tray_icon];
291291
callback_data->double_click_callback = callback;
292292
callback_data->double_click_user_data = user_data;
293293

294294
if (callback) {
295-
tray_icon_ptr->SetOnDoubleClick([tray_icon, callback_data = callback_data.get()]() {
296-
if (callback_data->double_click_callback) {
295+
tray_icon_ptr->SetOnDoubleClick([callback_data]() {
296+
if (callback_data && callback_data->double_click_callback) {
297297
callback_data->double_click_callback(callback_data->double_click_user_data);
298298
}
299299
});

src/platform/macos/tray_icon_macos.mm

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,12 @@ - (void)statusItemRightClicked:(id)sender;
7272
}
7373

7474
TrayIcon::~TrayIcon() {
75+
// Clear callbacks to prevent dangling function objects
76+
if (pimpl_) {
77+
pimpl_->on_left_click_ = nullptr;
78+
pimpl_->on_right_click_ = nullptr;
79+
pimpl_->on_double_click_ = nullptr;
80+
}
7581
delete pimpl_;
7682
}
7783

@@ -255,19 +261,31 @@ - (void)statusItemRightClicked:(id)sender;
255261
// Internal method to handle click events
256262
void TrayIcon::HandleLeftClick() {
257263
if (pimpl_->on_left_click_) {
258-
pimpl_->on_left_click_();
264+
try {
265+
pimpl_->on_left_click_();
266+
} catch (...) {
267+
// Protect against callback exceptions
268+
}
259269
}
260270
}
261271

262272
void TrayIcon::HandleRightClick() {
263273
if (pimpl_->on_right_click_) {
264-
pimpl_->on_right_click_();
274+
try {
275+
pimpl_->on_right_click_();
276+
} catch (...) {
277+
// Protect against callback exceptions
278+
}
265279
}
266280
}
267281

268282
void TrayIcon::HandleDoubleClick() {
269283
if (pimpl_->on_double_click_) {
270-
pimpl_->on_double_click_();
284+
try {
285+
pimpl_->on_double_click_();
286+
} catch (...) {
287+
// Protect against callback exceptions
288+
}
271289
}
272290
}
273291

0 commit comments

Comments
 (0)