Skip to content

safety: copy-then-use raw pointer after lock drop is fragile and needs invariant documentation #41

@rmzi

Description

@rmzi

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions