Skip to content
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.

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

Make handles Send + Sync #152

Closed
wants to merge 1 commit into from
Closed

Make handles Send + Sync #152

wants to merge 1 commit into from

Conversation

madsmtm
Copy link
Member

@madsmtm madsmtm commented Oct 13, 2023

In winit we're currently discussing this, both in rust-windowing/winit#3136 and on Matrix. See also previous discussion in #59, and the related issue in #85.

So I've opened this PR to see what it would take to make the handles Send + Sync, which would be somewhat possible on macOS, iOS and the web (provided that users uphold the "only use this on the main thread" requirement, which one could argue might be a safety footgun) - though I'm unsure if it's actually safe on the other platforms?

In any case we need to do something, because winit::Window is Send + Sync, and it unconditionally implements HasWindowHandle, so users could always just send the window to another thread and get a handle from that.

Fixes #85 and fixes #147.

TODO:

  • Changelog entry
  • Fix todos about safety comments

@madsmtm madsmtm added bug Something isn't working enhancement New feature or request labels Oct 13, 2023
@kchibisov
Copy link
Member

We had a lengthy discussion with @daxpedda on matrix, and I believe we end up with a solution different to what you propose.

The thing is that we must not mark it as a Send + Sync, since it's a footgun, and in reality it's not a Send + Sync on all the platforms. We can mark individual handlers as Send + Sync, but we can't guarantee that the handler will be Send + Sync all the time.

In any case we need to do something, because winit::Window is Send + Sync, and it unconditionally implements HasWindowHandle, so users could always just send the window to another thread and get a handle from that.

The solution to that was to get rid of the handler traits or split getting a handlers out of some pseudo struct (has no use other than give you handle based on thread). In the end, what winit will have is a regular raw_window_handle getter, but which will fail if you call to it
on a thread other than event loop's thread, and an unsafe method, which will work, but at least there will be an attempt to deliver what you need to do and it'll that, while you have handler, normal reading/writing won't work like on the main thread, and you'll need extra sync.

Also, making something Send and Sync when in reality they are not doesn't make any sense, because users can clearly just wrap the type and send across the thread boundaries.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

Should RawWindowHandle implement Send? Add tests for Rust subleties
2 participants