impl Sync for SerialPort trait
#233
Conversation
src/windows/com.rs
Outdated
| // The Windows [`HANDLE`] type is considered safe according to the standard library. | ||
| // See explanation on [`OwnedHandle`] in stdlib for more information. |
There was a problem hiding this comment.
Looking at https://doc.rust-lang.org/std/os/windows/raw/type.HANDLE.html or https://doc.rust-lang.org/std/os/windows/io/struct.OwnedHandle.html, I don't see the documentation you are mentioning at al first glance. Where is the statement made you are referring to? In this comment in handle.rs
There was a problem hiding this comment.
Yes, I was referring to the comment in handle.rs that you linked.
I'm not sure how to link it to an impl block so I figured the definition of OwnedHandle might be the closest to that comment (50+ lines away)
The link to the std source https://doc.rust-lang.org/src/std/os/windows/io/handle.rs.html#111-117 doesn't seem to be a permanent one so I was worried putting the doc link in might become invalid in the future.
I can probably replace it with a GitHub permalink like this: https://github.com/rust-lang/rust/blob/f4f0fafd0c7849e162eddbc69fa5fe82dbec28c7/library/std/src/os/windows/io/handle.rs#L111-L115
but that would be a very long line, unfortunately.
There was a problem hiding this comment.
Great, then we are on the same page. A permalink would be great then because a long line looks way more appealing to me than the risk of getting it broken at some point in time.
There was a problem hiding this comment.
Thanks for your feedback! I've now updated the patch with a permalink to the explanation.
I was trying to display some port information and settings that should only require an immutable reference in another thread. Without |
Closes #232
There are many alternatives that don't involve
unsafe impl, but I believe this is the simplest one that doesn't involve an MSRV bump, nor should it introduce breakage, at least I hope...One alternative is to use
OwnedHandleprovided by stdlib. However, doing so requires an MSRV bump to at least 1.63.0, which I've heard would require a major version bump in this library so I refrained from this approach.