Implement Send and Sync for HANDLE

[JunkuiZhang]

Suggestion

In #3111, windows-rs changes HANDLE, HMONITOR, and other similar types from isize to c_void. From my limited knowledge of the Win32 API, I think that HANDLE, HMONITOR, and similar structures should be thread-safe and can be safely sent across threads. Am I correct in this assumption? I apologize if this is a naive question.

If these types are thread-safe, can windows-rs impl Send and Sync for them? If not, what is the best practice for sending these pointers across threads? Thanks in advance.

[kennykerr]

See #3093 (comment)

[ReactorScram]

I'm using HANDLE for a registry listener, ofc this makes it incompatible with typical tokio code. Is there a better workaround than just moving it to its own worker thread? It's kind of a waste, maybe I can run a single-threaded async runtime on that worker and share it with some other Windows stuff that needs a worker, but I'm not sure.

[riverar]

@ChrisDenton @dpaoliello Thoughts on this? It's my understanding Sync/Send would indicate that access to the handle (non-pointer integer value) is thread-safe and does not indicate anything about usage with various APIs that may or may not be thread affine/etc. Following that understanding, it would seem to make sense that we blanket impl Send/Sync for all handle types.

[sivadeilra]

HANDLE and such are used in far too many ways to say definitively that they are Send or Sync. I think implementing Send or Sync on these values would likely do more harm than good.

It's a good question, but I think the answer is unfortunately a mixture of these factors:

1. These APIs were designed long before Rust's borrow-checker.
2. These HANDLE types are used in many different APIs for many different purposes. It's basically equivalent to void*, which is why it is typed that way.
3. We need to build safe abstractions that wrap types like HANDLE, not directly expose them. There should be many such safe abstractions, where each of them covers a specific use case. For example, opening files is different from opening registry keys, or handles to RPC objects.
4. In some cases, it's not clear that a handle even has a statically knowable safety property with respect to Send or Sync. In some cases, bit flags that are passed to "open" APIs (such as CreateFileW) affect whether a given handle can safely be used or moved between threads.

It's a good question, but unfortunately I think the answer is "there isn't a single good answer". The right answer is building safe abstractions on top of those types. Each of those safe abstractions can determine whether Send or Sync is appropriate for that abstraction.

[riverar]

My thinking was:

The Sync marker trait indicates that it is safe for the type implementing Sync to be referenced from multiple threads (docs). HANDLEs are just opaque integers (not pointers) so referencing them from multiple threads should be guaranteed safe. Usage of the data contained within with an API is, to my understanding, not in scope for Sync.

The Send marker trait indicates that ownership of values of the type implementing Send can be transferred between threads (docs). This one is murkier, as I believe it's possible an API could have returned a handle that is thread affine.

But if I zoom out and look at the concurrency experience that Rust is trying to sell, I think I agree with you.

[ChrisDenton]

A handle, in general, is a nebulous term that covers many different things and I'm not sure how well the metadata can distinguish. But if we're talking specifically about a kernel object type of handle then these should by themselves always be thread safe. Yes, things like FILE_FLAG_OVERLAPPED can complicate the safety story but that's true for single threaded applications too. E.g. if there's a buffer or OVERLAPPED structure on the stack then extra care needs to be taken not to return before an async operation completes. The correct solution here is to encapsulate all the required parts for the duration of the async op; between such operations it's safe to do what you please.

[sivadeilra]

The trouble is, HANDLE is used for far more than just NT kernel objects. It's used by plenty of public APIs that have nothing to do with kernel objects.

Sending the bit pattern of HANDLE across threads is uninteresting; you can always cast it to usize if you want. The important thing is what can you do with it? HANDLE is exactly as dangerous and untyped as void*; its meaning is always context-dependent.

[NuSkooler]

Apologies, but I was sent here from #3181 -- it seems this ticket is closed as well however. Which issue is correct for this?

[ReactorScram]

@NuSkooler I think they're both closed as "can't be fixed". In Firezone I just created a worker thread to solve it, since it's hard to prove that my code is thread-safe otherwise. :/ Can't do much for now

[kennykerr]

If you happen to know that your particular use of a handle is thread-safe then you can certainly wrap the handle in your own type that is Send and Sync to simplify/optimize your implementation. This thread safety just can't be guaranteed in the general case.

[NuSkooler]

@ReactorScram @kennykerr thanks for your quick response. In my particular case, it's with CreateEvent and friends.

I do wonder: Would it be fruitful to supply a generic wrapper struct that can be used along with some of these APIs?

[bryanlarsen]

In my case, I'm using a HANDLE from a CreateMutex. It's only purpose is to be sent between threads, so I need a workaround.

I tried creating a wrapper:

struct SafeHandle(HANDLE);
unsafe impl Send for SafeHandle {}
unsafe impl Sync for SafeHandle {}

...

        let shutdown_mutex = unsafe {
            CreateMutexW(
                0 as *const SECURITY_ATTRIBUTES,
                true as i32,
                0 as *const u16,
            )
        };
        let join_handle = Some({
            let shutdown_mutex = SafeHandle(shutdown_mutex.clone());
            std::thread::spawn(move || {

But I still get *mut c_void cannot be sent between threads safely`

[bryanlarsen]

I (hope) I found a solution:

            let shutdown_mutex = shutdown_mutex.clone() as isize;
            std::thread::spawn(move || {
...

[sivadeilra]

Casting to isize (or usize, as mentioned above) and then casting back to HANDLE is a valid way to pass it across threads. Remember, since you're working in unsafe code, it's up to you to follow the rules.