Skip to content

safety: TrayState unsafe Send/Sync impl needs explicit safety comment #42

@rmzi

Description

@rmzi

Description

The refactor consolidated five separate `OnceLock<Mutex<RawPtr>>` statics into a single `OnceLock<Mutex>`. The old code had a per-type `RawPtr` wrapper with its own `unsafe impl Send/Sync` and an explicit comment explaining the safety contract:

```rust
// NSWindow and NSView are not Send/Sync, so we store raw pointers.
// Safety: all access happens on the main thread (or via performSelectorOnMainThread).
struct RawPtr(*mut T);
unsafe impl Send for RawPtr {}
unsafe impl Sync for RawPtr {}
```

The new code has:

```rust
struct TrayState {
window: *mut NSWindow,
view: *mut AnyObject,
button: *mut AnyObject,
menu: *mut NSMenu,
status_item: *mut NSStatusItem,
}
unsafe impl Send for TrayState {}
unsafe impl Sync for TrayState {}
```

The `unsafe impl`s are present and correct, but the safety comment that explains why this is sound was dropped. The comment is load-bearing documentation: it tells reviewers what thread-safety contract is being upheld in lieu of the type system. Without it, a future change that accidentally uses a pointer off the main thread (e.g., directly calling `msg_send!` from the reader thread instead of routing through `performSelectorOnMainThread`) would not be flagged by any comment-level audit.

This is a documentation regression, not a behavioral one. The old `RawPtr` also made the intent per-pointer-type explicit; `TrayState` mixes five raw pointers into one struct without explaining each one's access protocol.

Location

  • `src/tray.rs:35-43` — `TrayState` definition and `unsafe impl` blocks

Desired State

Restore the safety comment above the `unsafe impl` blocks, e.g.:

```rust
// Safety: All five raw pointers point to AppKit objects that are not Send/Sync.
// Correctness relies on the invariant that:
// - window and view are only accessed on the main thread (via MainThreadMarker or
// performSelectorOnMainThread).
// - button, menu, status_item are AppKit objects accessed only on the main thread.
// - The Mutex ensures exclusive access when pointer values are read or written.
unsafe impl Send for TrayState {}
unsafe impl Sync for TrayState {}
```

Effort / Priority

small / low

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