Skip to content

fix(macos): release leaked window delegate Box and ObjC class on window close #1913

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ManthanNimodiya wants to merge 3 commits into
base: main
Choose a base branch
from
+86 −45
Open

fix(macos): release leaked window delegate Box and ObjC class on window close #1913

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c713ff1
fix(macos): release leaked window delegate state and reuse a single d…
ManthanNimodiya Jun 13, 2026
ddd6887
fix(macos): use alloc+new and restore prior delegate before release p…
ManthanNimodiya Jun 13, 2026
f6663a8
fix(macos): guard with_window_state against null app_box after window…
ManthanNimodiya Jun 13, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 86 additions & 45 deletions apps/desktop/src-tauri/src/platform/macos/delegates.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,7 @@
/// (Hoppscotch) https://github.com/hoppscotch/hoppscotch/blob/286fcd2bb08a84f027b10308d1e18da368f95ebf/packages/hoppscotch-selfhost-desktop/src-tauri/src/mac/window.rs
/// (Electron) https://github.com/electron/electron/blob/38512efd25a159ddc64a54c22ef9eb6dd60064ec/shell/browser/native_window_mac.mm#L1454
///
use objc::{msg_send, sel, sel_impl};
use rand::{Rng, distributions::Alphanumeric};
use objc::{class, msg_send, sel, sel_impl};
use tauri::{Emitter, LogicalPosition, Runtime, Window};

pub struct UnsafeWindowHandle(pub *mut std::ffi::c_void);
Expand Down Expand Up @@ -72,7 +71,8 @@ pub fn setup<R: Runtime>(window: Window<R>, controls_inset: LogicalPosition<f64>
use cocoa::appkit::NSWindow;
use cocoa::base::{BOOL, id};
use cocoa::foundation::NSUInteger;
use objc::runtime::{Object, Sel};
use objc::declare::ClassDecl;
use objc::runtime::{Class, Object, Sel};
use std::ffi::c_void;

let Ok(ns_win) = window.ns_window() else {
Expand All @@ -88,10 +88,13 @@ pub fn setup<R: Runtime>(window: Window<R>, controls_inset: LogicalPosition<f64>
this: &Object,
func: F,
) {
let ptr = unsafe {
let x: *mut c_void = *this.get_ivar("app_box");
&mut *(x as *mut WindowState<R>)
};
let x: *mut c_void = unsafe { *this.get_ivar("app_box") };
// `app_box` is nulled out in `windowWillClose:`; ignore any late
// callbacks that arrive after that instead of dereferencing null.
if x.is_null() {
return;
}
let ptr = unsafe { &mut *(x as *mut WindowState<R>) };
func(ptr);
}

Expand All @@ -118,10 +121,30 @@ pub fn setup<R: Runtime>(window: Window<R>, controls_inset: LogicalPosition<f64>
msg_send![super_del, windowShouldClose: sender]
})
}
extern "C" fn on_window_will_close(this: &Object, _cmd: Sel, notification: id) {
extern "C" fn on_window_will_close<R: Runtime>(this: &Object, _cmd: Sel, notification: id) {
suppress_delegate_panic("windowWillClose:", (), || unsafe {
let super_del: id = *this.get_ivar("super_delegate");
let _: () = msg_send![super_del, windowWillClose: notification];

// Drop the boxed `WindowState<R>` (and the `Window<R>` handle it holds)
// that was leaked via `Box::into_raw` when this delegate was created.
let app_box: *mut c_void = *this.get_ivar("app_box");
if !app_box.is_null() {
drop(Box::from_raw(app_box as *mut WindowState<R>));
let this_mut = this as *const Object as *mut Object;
(*this_mut).set_ivar("app_box", std::ptr::null_mut::<c_void>());

@tembo tembo Bot Jun 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that app_box can be set to null, any late delegate callback that still hits this object (e.g. an event queued before setDelegate: takes effect) will cause with_window_state to deref a null pointer and SIGSEGV.

Might be worth adding a null guard in with_window_state (or otherwise ensuring no callbacks can run after windowWillClose:).

}

// Restore the previous delegate before releasing this one, so any
// further delegate callbacks during teardown don't hit a freed object.
let window: id = *this.get_ivar("window");
let _: () = msg_send![window, setDelegate: super_del];

// NSWindow does not retain its delegate, so the reference taken when
// this delegate was created (`new`) is the only owning one. Release
// it now that the window is closing.
let this_id = this as *const Object as id;
let _: () = msg_send![this_id, release];
});
}
Comment on lines +124 to 149

@tembo tembo Bot Jun 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor robustness nit: right now all of the cleanup lives inside the same suppress_delegate_panic closure as the forwarded super_del call. If msg_send![super_del, windowWillClose: …] ever panics, we’ll skip dropping app_box / restoring the delegate / releasing self, which brings back the leak in the error path.

Suggested change
extern "C" fn on_window_will_close<R: Runtime>(this: &Object, _cmd: Sel, notification: id) {
suppress_delegate_panic("windowWillClose:", (), || unsafe {
let super_del: id = *this.get_ivar("super_delegate");
let _: () = msg_send![super_del, windowWillClose: notification];
// Drop the boxed `WindowState<R>` (and the `Window<R>` handle it holds)
// that was leaked via `Box::into_raw` when this delegate was created.
let app_box: *mut c_void = *this.get_ivar("app_box");
if !app_box.is_null() {
drop(Box::from_raw(app_box as *mut WindowState<R>));
let this_mut = this as *const Object as *mut Object;
(*this_mut).set_ivar("app_box", std::ptr::null_mut::<c_void>());
}
// Restore the previous delegate before releasing this one, so any
// further delegate callbacks during teardown don't hit a freed object.
let window: id = *this.get_ivar("window");
let _: () = msg_send![window, setDelegate: super_del];
// NSWindow does not retain its delegate, so the reference taken when
// this delegate was created (`new`) is the only owning one. Release
// it now that the window is closing.
let this_id = this as *const Object as id;
let _: () = msg_send![this_id, release];
});
}
extern "C" fn on_window_will_close<R: Runtime>(this: &Object, _cmd: Sel, notification: id) {
let super_del: id = unsafe { *this.get_ivar("super_delegate") };
// Forward to the previous delegate, but don't let it prevent cleanup.
suppress_delegate_panic("windowWillClose:", (), || unsafe {
let _: () = msg_send![super_del, windowWillClose: notification];
});
suppress_delegate_panic("windowWillClose:cleanup", (), || unsafe {
// Drop the boxed `WindowState<R>` (and the `Window<R>` handle it holds)
// that was leaked via `Box::into_raw` when this delegate was created.
let app_box: *mut c_void = *this.get_ivar("app_box");
if !app_box.is_null() {
drop(Box::from_raw(app_box as *mut WindowState<R>));
let this_mut = this as *const Object as *mut Object;
(*this_mut).set_ivar("app_box", std::ptr::null_mut::<c_void>());
}
// Restore the previous delegate before releasing this one, so any
// further delegate callbacks during teardown don't hit a freed object.
let window: id = *this.get_ivar("window");
let _: () = msg_send![window, setDelegate: super_del];
// NSWindow does not retain its delegate, so the reference taken when
// this delegate was created (`new`) is the only owning one. Release
// it now that the window is closing.
let this_id = this as *const Object as id;
let _: () = msg_send![this_id, release];
});
}

extern "C" fn on_window_did_resize<R: Runtime>(this: &Object, _cmd: Sel, notification: id) {
Expand Down Expand Up @@ -329,48 +352,66 @@ pub fn setup<R: Runtime>(window: Window<R>, controls_inset: LogicalPosition<f64>
);
}

let window_label = window.label().to_string();
// Register the delegate class once and reuse it for every window. Previously a brand
// new class was registered (with a randomized name) on every call to `setup`, which
// permanently leaked Objective-C class metadata for the lifetime of the process.
//
// NOTE: `static CLASS` below is a single process-wide instance shared across every
// monomorphization of this function, not one per `R`. `setup` is only ever called
// with `R = tauri::Wry` in this app, so this is fine in practice; if it were ever
// called with a different `R`, the first call's `on_*::<R>` method pointers would
// be baked into the shared class for all `R`.
fn get_or_register_delegate_class<R: Runtime>() -> &'static Class {

@tembo tembo Bot Jun 13, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heads up: since get_or_register_delegate_class is generic, the static CLASS is per-monomorphization, but the ObjC class name is always CapWindowDelegate. If setup() is ever called with a different R, this will try to re-register the same class name and panic.

Probably fine if there’s only ever one runtime type in practice, but if not, you may want a stable per-R class name (e.g., hash type_name::<R>()) or otherwise enforce a single R.

static CLASS: std::sync::OnceLock<&'static Class> = std::sync::OnceLock::new();
*CLASS.get_or_init(|| {
let mut decl = ClassDecl::new("CapWindowDelegate", class!(NSObject))
.expect("CapWindowDelegate class already registered");

decl.add_ivar::<id>("window");
decl.add_ivar::<*mut c_void>("app_box");
decl.add_ivar::<id>("toolbar");
decl.add_ivar::<id>("super_delegate");

unsafe {
decl.add_method(sel!(windowShouldClose:), on_window_should_close as extern "C" fn(&Object, Sel, id) -> BOOL);
decl.add_method(sel!(windowWillClose:), on_window_will_close::<R> as extern "C" fn(&Object, Sel, id));
decl.add_method(sel!(windowDidResize:), on_window_did_resize::<R> as extern "C" fn(&Object, Sel, id));
decl.add_method(sel!(windowDidMove:), on_window_did_move as extern "C" fn(&Object, Sel, id));
decl.add_method(sel!(windowDidChangeBackingProperties:), on_window_did_change_backing_properties as extern "C" fn(&Object, Sel, id));
decl.add_method(sel!(windowDidBecomeKey:), on_window_did_become_key as extern "C" fn(&Object, Sel, id));
decl.add_method(sel!(windowDidResignKey:), on_window_did_resign_key as extern "C" fn(&Object, Sel, id));
decl.add_method(sel!(draggingEntered:), on_dragging_entered as extern "C" fn(&Object, Sel, id) -> BOOL);
decl.add_method(sel!(prepareForDragOperation:), on_prepare_for_drag_operation as extern "C" fn(&Object, Sel, id) -> BOOL);
decl.add_method(sel!(performDragOperation:), on_perform_drag_operation as extern "C" fn(&Object, Sel, id) -> BOOL);
decl.add_method(sel!(concludeDragOperation:), on_conclude_drag_operation as extern "C" fn(&Object, Sel, id));
decl.add_method(sel!(draggingExited:), on_dragging_exited as extern "C" fn(&Object, Sel, id));
decl.add_method(sel!(window:willUseFullScreenPresentationOptions:), on_window_will_use_full_screen_presentation_options as extern "C" fn(&Object, Sel, id, NSUInteger) -> NSUInteger);
decl.add_method(sel!(windowDidEnterFullScreen:), on_window_did_enter_full_screen::<R> as extern "C" fn(&Object, Sel, id));
decl.add_method(sel!(windowWillEnterFullScreen:), on_window_will_enter_full_screen::<R> as extern "C" fn(&Object, Sel, id));
decl.add_method(sel!(windowDidExitFullScreen:), on_window_did_exit_full_screen::<R> as extern "C" fn(&Object, Sel, id));
decl.add_method(sel!(windowWillExitFullScreen:), on_window_will_exit_full_screen::<R> as extern "C" fn(&Object, Sel, id));
decl.add_method(sel!(windowDidFailToEnterFullScreen:), on_window_did_fail_to_enter_full_screen as extern "C" fn(&Object, Sel, id));
decl.add_method(sel!(effectiveAppearanceDidChange:), on_effective_appearance_did_change as extern "C" fn(&Object, Sel, id));
decl.add_method(sel!(effectiveAppearanceDidChangedOnMainThread:), on_effective_appearance_did_changed_on_main_thread as extern "C" fn(&Object, Sel, id));
}

decl.register()
})
}
Comment on lines +364 to +400

@greptile-apps greptile-apps Bot Jun 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Shared static CLASS captures function pointers for only the first R monomorphization

In Rust, static items inside generic functions are shared across all monomorphizations — there is exactly one CLASS for the entire program regardless of R. get_or_init will only ever run the closure once, registering ObjC method implementations for whichever R type first calls setup. Any subsequent setup::<DifferentR>() reuses that class silently, with the wrong monomorphized function pointers. In a real Tauri application R is always tauri::Wry, so this causes no observable issue today, but the pattern will silently misbehave if the type ever changes.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: apps/desktop/src-tauri/src/platform/macos/delegates.rs
Line: 350-386

Comment:
**Shared `static CLASS` captures function pointers for only the first `R` monomorphization**

In Rust, `static` items inside generic functions are shared across all monomorphizations — there is exactly one `CLASS` for the entire program regardless of `R`. `get_or_init` will only ever run the closure once, registering ObjC method implementations for whichever `R` type first calls `setup`. Any subsequent `setup::<DifferentR>()` reuses that class silently, with the wrong monomorphized function pointers. In a real Tauri application `R` is always `tauri::Wry`, so this causes no observable issue today, but the pattern will silently misbehave if the type ever changes.

How can I resolve this? If you propose a fix, please make it concise.


let app_state = WindowState {
window,
controls_inset,
};
let app_box = Box::into_raw(Box::new(app_state)) as *mut c_void;
let random_str: String = rand::thread_rng()
.sample_iter(&Alphanumeric)
.take(20)
.map(char::from)
.collect();

// We need to ensure we have a unique delegate name, otherwise we will panic while trying to create a duplicate
// delegate with the same name.
let delegate_name = format!("windowDelegate_cap_{window_label}_{random_str}");

ns_win_id.setDelegate_(cocoa::delegate!(&delegate_name, {
window: id = ns_win_id,
app_box: *mut c_void = app_box,
toolbar: id = cocoa::base::nil,
super_delegate: id = current_delegate,
(windowShouldClose:) => on_window_should_close as extern "C" fn(&Object, Sel, id) -> BOOL,
(windowWillClose:) => on_window_will_close as extern "C" fn(&Object, Sel, id),
(windowDidResize:) => on_window_did_resize::<R> as extern "C" fn(&Object, Sel, id),
(windowDidMove:) => on_window_did_move as extern "C" fn(&Object, Sel, id),
(windowDidChangeBackingProperties:) => on_window_did_change_backing_properties as extern "C" fn(&Object, Sel, id),
(windowDidBecomeKey:) => on_window_did_become_key as extern "C" fn(&Object, Sel, id),
(windowDidResignKey:) => on_window_did_resign_key as extern "C" fn(&Object, Sel, id),
(draggingEntered:) => on_dragging_entered as extern "C" fn(&Object, Sel, id) -> BOOL,
(prepareForDragOperation:) => on_prepare_for_drag_operation as extern "C" fn(&Object, Sel, id) -> BOOL,
(performDragOperation:) => on_perform_drag_operation as extern "C" fn(&Object, Sel, id) -> BOOL,
(concludeDragOperation:) => on_conclude_drag_operation as extern "C" fn(&Object, Sel, id),
(draggingExited:) => on_dragging_exited as extern "C" fn(&Object, Sel, id),
(window:willUseFullScreenPresentationOptions:) => on_window_will_use_full_screen_presentation_options as extern "C" fn(&Object, Sel, id, NSUInteger) -> NSUInteger,
(windowDidEnterFullScreen:) => on_window_did_enter_full_screen::<R> as extern "C" fn(&Object, Sel, id),
(windowWillEnterFullScreen:) => on_window_will_enter_full_screen::<R> as extern "C" fn(&Object, Sel, id),
(windowDidExitFullScreen:) => on_window_did_exit_full_screen::<R> as extern "C" fn(&Object, Sel, id),
(windowWillExitFullScreen:) => on_window_will_exit_full_screen::<R> as extern "C" fn(&Object, Sel, id),
(windowDidFailToEnterFullScreen:) => on_window_did_fail_to_enter_full_screen as extern "C" fn(&Object, Sel, id),
(effectiveAppearanceDidChange:) => on_effective_appearance_did_change as extern "C" fn(&Object, Sel, id),
(effectiveAppearanceDidChangedOnMainThread:) => on_effective_appearance_did_changed_on_main_thread as extern "C" fn(&Object, Sel, id)
}))

let delegate_class = get_or_register_delegate_class::<R>();
let delegate: id = msg_send![delegate_class, new];
(*delegate).set_ivar("window", ns_win_id);
Comment on lines +408 to +410

@greptile-apps greptile-apps Bot Jun 13, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Missing init after alloc

ObjC convention requires alloc to be followed by init before any instance methods (including ivar access) are used. For a plain NSObject subclass init is a no-op and alloc zeroes memory, so this works today, but it is technically undefined per the ObjC spec and may break with stricter runtimes or future changes.

Suggested change
let delegate_class = get_or_register_delegate_class::<R>();
let delegate: id = msg_send![delegate_class, alloc];
(*delegate).set_ivar("window", ns_win_id);
let delegate_class = get_or_register_delegate_class::<R>();
let delegate: id = {
let alloc: id = msg_send![delegate_class, alloc];
msg_send![alloc, init]
};
(*delegate).set_ivar("window", ns_win_id);
Prompt To Fix With AI 
This is a comment left during a code review.
Path: apps/desktop/src-tauri/src/platform/macos/delegates.rs
Line: 394-396

Comment:
**Missing `init` after `alloc`**

ObjC convention requires `alloc` to be followed by `init` before any instance methods (including ivar access) are used. For a plain `NSObject` subclass `init` is a no-op and `alloc` zeroes memory, so this works today, but it is technically undefined per the ObjC spec and may break with stricter runtimes or future changes.

```suggestion
        let delegate_class = get_or_register_delegate_class::<R>();
        let delegate: id = {
            let alloc: id = msg_send![delegate_class, alloc];
            msg_send![alloc, init]
        };
        (*delegate).set_ivar("window", ns_win_id);
```

How can I resolve this? If you propose a fix, please make it concise.

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

(*delegate).set_ivar("app_box", app_box);
(*delegate).set_ivar("toolbar", cocoa::base::nil);
(*delegate).set_ivar("super_delegate", current_delegate);

ns_win_id.setDelegate_(delegate)
}
}
Loading