linebender / glazier

Deprecated Rust Window Creation Library
Apache License 2.0
206 stars 32 forks source link

Windows: `HCursor` not `Send`/`Sync`, but used with `Arc` #138

Closed waywardmonkeys closed 1 year ago

waywardmonkeys commented 1 year ago

Running a nightly clippy, this gets emitted:

warning: usage of an `Arc` that is not `Send` or `Sync`
    --> src\backend\windows\window.rs:1965:50
     |
1965 |                 Some(Cursor::Custom(CustomCursor(Arc::new(HCursor(icon)))))
     |                                                  ^^^^^^^^^^^^^^^^^^^^^^^
     |
     = note: the trait `Send` is not implemented for `HCursor`
     = note: the trait `Sync` is not implemented for `HCursor`
     = note: required for `Arc<HCursor>` to implement `Send` and `Sync`
     = help: consider using an `Rc` instead or wrapping the inner type with a `Mutex`
     = help: for further information visit https://rust-lang.github.io/rust-clippy/master/index.html#arc_with_non_send_sync
     = note: `#[warn(clippy::arc_with_non_send_sync)]` on by default

Maybe the check is wrong, I haven't looked.

The X11 code is the only one other backend with an interesting CustomCursor impl (non-empty), but it doesn't seem to care about threads or refcounting. So I'm curious what the threading model is here...

waywardmonkeys commented 1 year ago

That lint is coming in 1.72, so not all that far away in time...

DJMcNab commented 1 year ago

I'd be most inclined to just make Cursor not be Send/Sync, and so use Rc here. Especially if we migrate to https://github.com/rust-windowing/cursor-icon as mentioned in #136, that can be passed around as the Sync version if people need it.

That matches the rest of our API, and the custom cursor variants are completely useless not on the main thread anyway.

waywardmonkeys commented 1 year ago

If no one says otherwise, I'll tackle this sometime this week.