Skip to content

Commit 0cd269e

Browse files
committed
Refactor global registry and add tray icon cleanup
Moved global registry template to a new header for shared use and updated menu and tray icon C APIs to use it. Added explicit cleanup functions for tray icon resources and documented their usage. Improved resource management and clarified object lifetime handling.
1 parent 9b58825 commit 0cd269e

File tree

6 files changed

+70
-39
lines changed

6 files changed

+70
-39
lines changed

src/capi/menu_c.cpp

Lines changed: 2 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,12 @@
44
#include <cstring>
55
#include <map>
66
#include <memory>
7-
#include <unordered_map>
7+
#include <optional>
8+
#include "../global_registry.h"
89
#include "../menu.h"
910

1011
using namespace nativeapi;
1112

12-
// Template function to create and manage global registries for different object
13-
// types Uses heap allocation to avoid destruction order issues at program exit
14-
template <typename T>
15-
static auto& globalRegistry() {
16-
static auto* instance = new std::unordered_map<void*, std::shared_ptr<T>>();
17-
return *instance;
18-
}
19-
2013
// Global registry instances for managing MenuItem and Menu object lifetimes
2114
static auto& g_menu_items = globalRegistry<MenuItem>();
2215
static auto& g_menus = globalRegistry<Menu>();
@@ -1144,23 +1137,6 @@ void native_menu_item_list_free(native_menu_item_list_t list) {
11441137
}
11451138
}
11461139

1147-
// Implementation of cleanup function
1148-
void cleanup_at_exit() {
1149-
// Clear all event listeners first to prevent callbacks during cleanup
1150-
g_menu_item_listeners.clear();
1151-
g_menu_listeners.clear();
1152-
1153-
// Note: We intentionally do NOT clear g_menu_items and g_menus here
1154-
// because they are now managed by function-local static pointers that
1155-
// won't be destroyed during normal exit, preventing double-free issues.
1156-
// The memory will be cleaned up by the OS when the process terminates.
1157-
}
1158-
1159-
// Cleanup functions to ensure proper resource management
1160-
void native_menu_cleanup_all(void) {
1161-
cleanup_at_exit();
1162-
}
1163-
11641140
char* native_keyboard_accelerator_to_string(
11651141
const native_keyboard_accelerator_t* accelerator) {
11661142
if (!accelerator)

src/capi/tray_icon_c.cpp

Lines changed: 36 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
#include <cstring>
33
#include <map>
44
#include <memory>
5+
#include <optional>
6+
#include "../global_registry.h"
57
#include "../tray_icon.h"
68
#include "../tray_icon_event.h"
79
#include "string_utils_c.h"
@@ -22,12 +24,17 @@ static std::map<native_tray_icon_t,
2224
g_tray_icon_listeners;
2325
static std::atomic<int> g_tray_icon_next_listener_id{1};
2426

27+
// Global registry instance for managing TrayIcon object lifetimes
28+
static auto& g_tray_icons = globalRegistry<TrayIcon>();
29+
2530
// TrayIcon C API Implementation
2631

2732
native_tray_icon_t native_tray_icon_create(void) {
2833
try {
2934
auto tray_icon = std::make_shared<TrayIcon>();
30-
return static_cast<native_tray_icon_t>(tray_icon.get());
35+
void* handle = tray_icon.get();
36+
g_tray_icons[handle] = tray_icon; // Store shared_ptr to keep object alive
37+
return static_cast<native_tray_icon_t>(handle);
3138
} catch (...) {
3239
return nullptr;
3340
}
@@ -39,7 +46,9 @@ native_tray_icon_t native_tray_icon_create_from_native(void* native_tray) {
3946

4047
try {
4148
auto tray_icon = std::make_shared<TrayIcon>(native_tray);
42-
return static_cast<native_tray_icon_t>(tray_icon.get());
49+
void* handle = tray_icon.get();
50+
g_tray_icons[handle] = tray_icon; // Store shared_ptr to keep object alive
51+
return static_cast<native_tray_icon_t>(handle);
4352
} catch (...) {
4453
return nullptr;
4554
}
@@ -49,14 +58,17 @@ void native_tray_icon_destroy(native_tray_icon_t tray_icon) {
4958
if (!tray_icon)
5059
return;
5160

52-
// Clean up listeners
53-
auto it = g_tray_icon_listeners.find(tray_icon);
54-
if (it != g_tray_icon_listeners.end()) {
55-
g_tray_icon_listeners.erase(it);
61+
// Remove event listeners first
62+
auto listeners_it = g_tray_icon_listeners.find(tray_icon);
63+
if (listeners_it != g_tray_icon_listeners.end()) {
64+
g_tray_icon_listeners.erase(listeners_it);
5665
}
5766

58-
// Note: The actual TrayIcon object is managed by shared_ptr
59-
// This just removes our reference to it
67+
// Remove from global storage - this will release the shared_ptr
68+
auto tray_icon_it = g_tray_icons.find(tray_icon);
69+
if (tray_icon_it != g_tray_icons.end()) {
70+
g_tray_icons.erase(tray_icon_it);
71+
}
6072
}
6173

6274
native_tray_icon_id_t native_tray_icon_get_id(native_tray_icon_t tray_icon) {
@@ -369,3 +381,19 @@ bool native_tray_icon_close_context_menu(native_tray_icon_t tray_icon) {
369381
return false;
370382
}
371383
}
384+
385+
// Implementation of cleanup function
386+
void cleanup_tray_icon_at_exit() {
387+
// Clear all event listeners first to prevent callbacks during cleanup
388+
g_tray_icon_listeners.clear();
389+
390+
// Note: We intentionally do NOT clear g_tray_icons here
391+
// because they are now managed by function-local static pointers that
392+
// won't be destroyed during normal exit, preventing double-free issues.
393+
// The memory will be cleaned up by the OS when the process terminates.
394+
}
395+
396+
// Cleanup functions to ensure proper resource management
397+
void native_tray_icon_cleanup_all(void) {
398+
cleanup_tray_icon_at_exit();
399+
}

src/capi/tray_icon_c.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -231,6 +231,13 @@ bool native_tray_icon_open_context_menu(native_tray_icon_t tray_icon);
231231
FFI_PLUGIN_EXPORT
232232
bool native_tray_icon_close_context_menu(native_tray_icon_t tray_icon);
233233

234+
/**
235+
* Cleanup all tray icon resources
236+
* This should be called at program exit to ensure proper cleanup
237+
*/
238+
FFI_PLUGIN_EXPORT
239+
void native_tray_icon_cleanup_all(void);
240+
234241
#ifdef __cplusplus
235242
}
236243
#endif

src/global_registry.h

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
#pragma once
2+
3+
#include <memory>
4+
#include <unordered_map>
5+
6+
namespace nativeapi {
7+
8+
// Template function to create and manage global registries for different object
9+
// types Uses heap allocation to avoid destruction order issues at program exit
10+
template <typename T>
11+
static auto& globalRegistry() {
12+
static auto* instance = new std::unordered_map<void*, std::shared_ptr<T>>();
13+
return *instance;
14+
}
15+
16+
} // namespace nativeapi

src/platform/macos/menu_macos.mm

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -597,7 +597,10 @@ - (void)menuDidClose:(NSMenu*)menu {
597597
bool visible_;
598598

599599
Impl(NSMenu* menu)
600-
: ns_menu_(menu), delegate_([[NSMenuDelegateImpl alloc] init]), enabled_(true), visible_(false) {
600+
: ns_menu_(menu),
601+
delegate_([[NSMenuDelegateImpl alloc] init]),
602+
enabled_(true),
603+
visible_(false) {
601604
[ns_menu_ setDelegate:delegate_];
602605
}
603606

src/platform/macos/tray_icon_macos.mm

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,6 @@ - (void)statusItemClicked:(id)sender;
2727
// Private implementation class
2828
class TrayIcon::Impl {
2929
public:
30-
Impl() : ns_status_item_(nil), ns_status_bar_button_target_(nil), menu_closed_listener_id_(0) {}
31-
3230
Impl(NSStatusItem* status_item)
3331
: ns_status_item_(status_item),
3432
ns_status_bar_button_target_(nil),
@@ -88,15 +86,15 @@ - (void)statusItemClicked:(id)sender;
8886
size_t menu_closed_listener_id_;
8987
};
9088

91-
TrayIcon::TrayIcon() : pimpl_(std::make_unique<Impl>()) {
89+
TrayIcon::TrayIcon() {
9290
id = -1;
9391

9492
// Create platform-specific NSStatusItem
9593
NSStatusBar* status_bar = [NSStatusBar systemStatusBar];
9694
NSStatusItem* status_item = [status_bar statusItemWithLength:NSVariableStatusItemLength];
9795

9896
if (status_item) {
99-
// Reinitialize the Impl with the created status item
97+
// Initialize the Impl with the created status item
10098
pimpl_ = std::make_unique<Impl>(status_item);
10199

102100
if (pimpl_->ns_status_bar_button_target_) {
@@ -128,6 +126,9 @@ - (void)statusItemClicked:(id)sender;
128126
}
129127
};
130128
}
129+
} else {
130+
// If status_item creation failed, create Impl with nil
131+
pimpl_ = std::make_unique<Impl>(nil);
131132
}
132133
}
133134

0 commit comments

Comments
 (0)