-
Notifications
You must be signed in to change notification settings - Fork 47
Description
Description
Several functions in `src/tray.rs` copy a raw pointer out of the `TRAY` mutex and then use that pointer after the lock is already dropped. Example:
```rust
// signal_redraw (line 436)
let ptr = TRAY.get().unwrap().lock().unwrap().view;
// MutexGuard dropped here — lock released
if !ptr.is_null() {
unsafe {
let _: () = msg_send![ptr, performSelectorOnMainThread: ...];
}
}
```
The same pattern appears in `dispatch_hide_window`, `position_window_below_icon`, and `toggle_window`.
In the current code this is safe because the pointer fields in `TrayState` are written once (during `ensure_window`/`run_tray`) and never set back to null — the objects are intentionally leaked via `mem::forget`. But this invariant is not documented and is not enforced by the type system. A future change that adds pointer invalidation (e.g., a `destroy_window` path or cleanup on quit) would make these sites silently become use-after-free bugs.
Location
- `src/tray.rs:436` — `signal_redraw`
- `src/tray.rs:445` — `dispatch_hide_window`
- `src/tray.rs:458` — `position_window_below_icon`
- `src/tray.rs:591` — `toggle_window`
Desired State
Add `// SAFETY:` comments at each copy-then-use site explicitly stating: "pointer fields are written once and never invalidated; the object's lifetime is managed by `mem::forget` in `ensure_window`". This makes the invariant visible to future maintainers and prevents silent regression if a cleanup path is ever added.
Effort / Priority
small / medium