Make Win32WindowHandle Send + Sync#209
Make Win32WindowHandle Send + Sync#209daxpedda wants to merge 3 commits intorust-windowing:masterfrom
Win32WindowHandle Send + Sync#209Conversation
b18e787 to
06aa544
Compare
| /// Raw window handle for Win32. | ||
| #[non_exhaustive] | ||
| #[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] | ||
| pub struct Win32WindowHandle { |
There was a problem hiding this comment.
Could we add an example of getting this, and compare it like:
assert_eq!(GetWindowThreadProcessId(hwnd, std::ptr::null_mut()), GetCurrentThreadId());Before doing anything with the handle? Just to make it clearer what consumers have to do.
There was a problem hiding this comment.
Done! I guess it would be nice if we actually run this test on Windows with all the function calls working. A problem for another time.
|
Also, would it make sense to wait for #188? Since the safety of this depends on what Then again, I'm also fine with doing this now (it's correct for the handle as currently defined), and then worry it this later when we do get around to changing the semantics of handles. See also #192, that one has a bit more detailed docs. |
See #197 (comment), but I expect we will hash this out in the next meeting. |
edd480a to
fa10f61
Compare
fa10f61 to
d304a61
Compare
| /// | ||
| /// ## Thread-Safety | ||
| /// | ||
| /// Overall, even though Win32 windows have [thread affinity], the overall | ||
| /// Win32 user API is thread-safe. Therefore this type is `Send` and `Sync`. | ||
| /// This means it can be sent to or used from other threads. | ||
| /// | ||
| /// [thread affinity]: https://devblogs.microsoft.com/oldnewthing/20051010-09/?p=33843 |
There was a problem hiding this comment.
I believe that this doesn't make sense here because it doesn't actually exist on Windows.
d304a61 to
b60b974
Compare
| /// | ||
| /// ## Thread-Safety | ||
| /// | ||
| /// Window handles have [thread affinity]. This means that they are `!Send`, as | ||
| /// they must be dropped on the same thread that created them. However, some | ||
| /// functions on the window can be called from other threads. This means that | ||
| /// the window is `Sync`. | ||
| /// | ||
| /// Note that not all functions of the Win32 handle are thread-safe (modifying | ||
| /// functions especially), so care should be taken to not call these functions | ||
| /// from other threads. When in doubt, only run the function on the main thread. | ||
| /// | ||
| /// [thread affinity]: https://devblogs.microsoft.com/oldnewthing/20051010-09/?p=33843 |
There was a problem hiding this comment.
Actually, CoreWindow is something called a "non-agile" type, which literally means it can't be sent or access in other threads. I could mention that instead but it feels unnecessary considering its part of the actual type and not something we artificially enforce.
WinRT has something called "agile reference"s. This allows you to do something like libdispatch on MacOS. We could introduce a new type WinRtAgileWindowHandle or just enforce this right here.
Resolves #196.