fix: preserve shortcut and window hook callback lifetimes

Clear shortcut callbacks after successful unregistration, outside the
manager mutex. Retained handles can no longer invoke stale callbacks.

Keep a local copy while invoking shortcuts and desktop window hooks so
self-replacement or removal does not destroy executing captures.

Add CTest coverage for self-replacement, hook removal, and retained handles.

Author: szymonmazanik

src/platform/linux/window_manager_linux.cpp

@@ -901,14 +901,18 @@ bool WindowManager::HasWillHideHook() const {
 }
 
 void WindowManager::HandleWillShow(WindowId id) {
-  if (pimpl_->will_show_hook_) {
-    (*pimpl_->will_show_hook_)(id);
+  // The hook may replace or clear itself. Keep this invocation alive.
+  auto hook = pimpl_->will_show_hook_;
+  if (hook) {
+    (*hook)(id);
   }
 }
 
 void WindowManager::HandleWillHide(WindowId id) {
-  if (pimpl_->will_hide_hook_) {
-    (*pimpl_->will_hide_hook_)(id);
+  // The hook may replace or clear itself. Keep this invocation alive.
+  auto hook = pimpl_->will_hide_hook_;
+  if (hook) {
+    (*hook)(id);
   }
 }
 

src/platform/macos/window_manager_macos.mm

@@ -526,14 +526,18 @@ static WindowId ResolveWindowId(NSWindow* ns_window) {
 }
 
 void WindowManager::HandleWillShow(WindowId id) {
-  if (pimpl_->will_show_hook_) {
-    (*pimpl_->will_show_hook_)(id);
+  // The hook may replace or clear itself. Keep this invocation alive.
+  auto hook = pimpl_->will_show_hook_;
+  if (hook) {
+    (*hook)(id);
   }
 }
 
 void WindowManager::HandleWillHide(WindowId id) {
-  if (pimpl_->will_hide_hook_) {
-    (*pimpl_->will_hide_hook_)(id);
+  // The hook may replace or clear itself. Keep this invocation alive.
+  auto hook = pimpl_->will_hide_hook_;
+  if (hook) {
+    (*hook)(id);
   }
 }
 

src/platform/windows/window_manager_windows.cpp

@@ -892,14 +892,18 @@ bool WindowManager::HasWillHideHook() const {
 }
 
 void WindowManager::HandleWillShow(WindowId id) {
-  if (pimpl_->will_show_hook_) {
-    (*pimpl_->will_show_hook_)(id);
+  // The hook may replace or clear itself. Keep this invocation alive.
+  auto hook = pimpl_->will_show_hook_;
+  if (hook) {
+    (*hook)(id);
   }
 }
 
 void WindowManager::HandleWillHide(WindowId id) {
-  if (pimpl_->will_hide_hook_) {
-    (*pimpl_->will_hide_hook_)(id);
+  // The hook may replace or clear itself. Keep this invocation alive.
+  auto hook = pimpl_->will_hide_hook_;
+  if (hook) {
+    (*hook)(id);
   }
 }
 

src/shortcut.cpp

@@ -49,8 +49,10 @@ bool Shortcut::IsEnabled() const {
 }
 
 void Shortcut::Invoke() {
-  if (enabled_ && callback_) {
-    callback_();
+  // A callback may replace or clear itself while it is executing.
+  auto callback = callback_;
+  if (enabled_ && callback) {
+    callback();
   }
 }
 

src/shortcut_manager.cpp

@@ -89,6 +89,11 @@ bool ShortcutManager::Unregister(ShortcutId id) {
   shortcuts_by_id_.erase(it);
   shortcuts_by_accelerator_.erase(accelerator);
 
+  // Destroying captured objects may re-enter the manager. Do it outside the lock.
+  lock.unlock();
+  // Retained handles must not invoke a callback after its registration ends.
+  shortcut->SetCallback(nullptr);
+
   // Emit event
   EmitAsync<ShortcutUnregisteredEvent>(id, accelerator);
 

tests/CMakeLists.txt

@@ -39,6 +39,11 @@ add_executable(shortcut_accelerator_test shortcut_accelerator_test.cpp)
 target_link_libraries(shortcut_accelerator_test PRIVATE nativeapi)
 add_test(NAME shortcut_accelerator_test COMMAND shortcut_accelerator_test)
 
+add_executable(callback_lifetime_test callback_lifetime_test.cpp)
+target_link_libraries(callback_lifetime_test PRIVATE nativeapi)
+add_test(NAME callback_lifetime_test COMMAND callback_lifetime_test)
+set_tests_properties(callback_lifetime_test PROPERTIES TIMEOUT 20)
+
 # The registry side of launch-at-login: the Run value, the StartupApproved flag that
 # Task Manager toggles, and a non-ASCII identifier. Writes only under HKCU, under a
 # test-only identifier, and removes what it writes.

tests/callback_lifetime_test.cpp

@@ -0,0 +1,104 @@
+// Callbacks must keep their captured objects alive while replacing or clearing
+// themselves. Unregistering a shortcut must release its callback even if the
+// caller still owns the shortcut.
+//
+// The registration case needs a desktop session and skips if the platform cannot
+// register the accelerator. The self-replacement cases run without registration.
+
+#include <cstdlib>
+#include <iostream>
+#include <memory>
+
+#include "../src/shortcut.h"
+#include "../src/shortcut_manager.h"
+#include "../src/window_manager.h"
+
+namespace {
+
+int g_failures = 0;
+
+void Check(bool condition, const char* message) {
+  if (!condition) {
+    std::cerr << "FAIL: " << message << std::endl;
+    ++g_failures;
+  }
+}
+
+void TestShortcutSelfReplacement() {
+  nativeapi::Shortcut shortcut(1, "Ctrl+A", nullptr);
+  auto captured = std::make_shared<int>(42);
+  std::weak_ptr<int> weak = captured;
+  int replacements = 0;
+  shortcut.SetCallback([&, captured] {
+    shortcut.SetCallback([&] { ++replacements; });
+    Check(!weak.expired(), "active shortcut callback was destroyed during replacement");
+  });
+  captured.reset();
+  shortcut.Invoke();
+  Check(weak.expired(), "replaced shortcut callback was retained");
+  shortcut.Invoke();
+  Check(replacements == 1, "replacement callback was not installed");
+  shortcut.SetCallback(nullptr);
+}
+
+void TestHookSelfRemoval() {
+  auto& manager = nativeapi::WindowManager::GetInstance();
+  for (bool show : {true, false}) {
+    auto captured = std::make_shared<int>(42);
+    std::weak_ptr<int> weak = captured;
+    auto callback = [&, captured](nativeapi::WindowId) {
+      if (show) {
+        manager.SetWillShowHook(std::nullopt);
+      } else {
+        manager.SetWillHideHook(std::nullopt);
+      }
+      Check(!weak.expired(), "active window hook was destroyed during removal");
+    };
+    if (show) {
+      manager.SetWillShowHook(std::move(callback));
+    } else {
+      manager.SetWillHideHook(std::move(callback));
+    }
+    captured.reset();
+    if (show) {
+      manager.HandleWillShow(0);
+    } else {
+      manager.HandleWillHide(0);
+    }
+    Check(weak.expired(), "removed window hook was retained");
+  }
+}
+
+void TestUnregisterClearsRetainedShortcut() {
+  auto& manager = nativeapi::ShortcutManager::GetInstance();
+  int calls = 0;
+  auto captured = std::make_shared<int>(42);
+  std::weak_ptr<int> weak = captured;
+  auto shortcut = manager.Register("Ctrl+Alt+Shift+F12", [&, captured] { ++calls; });
+  captured.reset();
+  if (!shortcut) {
+    std::cout << "SKIP: platform could not register a global shortcut" << std::endl;
+    return;
+  }
+  shortcut->Invoke();
+  Check(calls == 1, "registered shortcut callback did not run");
+  Check(manager.Unregister(shortcut->GetId()), "shortcut unregister failed");
+  shortcut->Invoke();
+  Check(calls == 1, "retained shortcut invoked after unregister");
+  Check(weak.expired(), "unregistered shortcut retained its captured state");
+}
+}  // namespace
+
+int main() {
+  std::cout << "callback_lifetime_test" << std::endl;
+
+  TestShortcutSelfReplacement();
+  TestHookSelfRemoval();
+  TestUnregisterClearsRetainedShortcut();
+  if (g_failures > 0) {
+    std::cerr << g_failures << " check(s) failed" << std::endl;
+    return EXIT_FAILURE;
+  }
+  std::cout << "All checks passed." << std::endl;
+  return EXIT_SUCCESS;
+}