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

RAII type for HANDLES? #2222

Closed
damyanp opened this issue Dec 3, 2022 · 11 comments
Closed

RAII type for HANDLES? #2222

damyanp opened this issue Dec 3, 2022 · 11 comments
Labels
question Further information is requested

Comments

@damyanp
Copy link
Member

damyanp commented Dec 3, 2022

Is there an RAII type for HANDLEs that'll call CloseHandle when dropped? As far as I can tell the current HANDLE requires an explicit call to CloseHandle.

@kennykerr
Copy link
Collaborator

No. The Win32 metadata, which defines HANDLE, doesn't have a reliable way to express ownership semantics. As you know, there are many uses of HANDLE, sometimes with different ways to free the handle, sometimes it is merely borrowed, sometimes representing a psuedo handle, etc. Ideally, the metadata would indicate such semantics on functions like CreateEventW to indicate what to do with the resulting handle.

@damyanp
Copy link
Member Author

damyanp commented Dec 3, 2022

Ok, good to know I wasn't missing something. Has this already been discussed? Is this in scope for v1.0?

@kennykerr
Copy link
Collaborator

Ideally, the Win32 metadata can get improved support for ownership semantics. @riverar and I have discussed this at some length. He may have some thoughts as well. But I probably have to come up with some way to infer lifetime/ownership.

This is a general problem, particularly challenging with DirectX APIs where there are structs with COM interface pointer fields and it's not always clear who owns what.

@riverar
Copy link
Collaborator

riverar commented Dec 4, 2022

Metadata has quite a bit of work ahead of it to make this work, or at least get it into a state where we can start experimentation.

For example, we're still tossing around candidates for how these attributes look microsoft/win32metadata#389. And we haven't nailed down a fix for free functions with multiple parameters microsoft/win32metadata#423. Or how to deal with functions that return either freeable and non-freeable resources based on other inputs microsoft/win32metadata#423 (comment).

@damyanp
Copy link
Member Author

damyanp commented Dec 5, 2022

It seems that there are several different problems some of affecting legacy APIs while others are affecting more modern ones. Is the idea to fix all at once, or can these be prioritized at all?

The D3D APIs I think should be pretty trivial - the COM pointers in D3D12 structs are all weak references.

@kennykerr kennykerr added the question Further information is requested label Dec 5, 2022
@kennykerr
Copy link
Collaborator

kennykerr commented Dec 5, 2022

The D3D APIs I think should be pretty trivial - the COM pointers in D3D12 structs are all weak references.

By weak reference I assume you don't mean a weak reference? 😉 I was going to apply some sort of borrow semantics to indicate that they're not owned. That seems almost universal for Win32 structs (but not WinRT structs) so I may just make that work universally, but lifetime may be hard to figure out so I don't know how practical that is (#1896) so worst case they may have to just be ManuallyDrop.

For APIs that return resources, like CreatEventW for example, we really need the API to provide a FreeWith attribute on the actual function rather than the returned type. There already appears to be a FreeWith attribute that is used in some cases e.g. IBackgroundCopyJobHttpOptions.GetClientCertificate, but we'd need it to be rolled out more uniformly. Also, the issues @riverar mentioned. 😊

@Chaoses-Ib
Copy link

Chaoses-Ib commented May 10, 2023

Here is a simple OwnedHandle:

use windows::Win32::Foundation::{HANDLE, CloseHandle};

/// You should only use this wrapper if the handle can be closed by `CloseHandle`.
#[derive(PartialEq, Eq, Debug)]
pub struct OwnedHandle(pub HANDLE);

impl From<HANDLE> for OwnedHandle {
    fn from(handle: HANDLE) -> Self {
        Self(handle)
    }
}

impl Drop for OwnedHandle {
    fn drop(&mut self) {
        unsafe { CloseHandle(self.0) };
    }
}

@MarijnS95
Copy link
Contributor

MarijnS95 commented Sep 6, 2023

@Chaoses-Ib (and @NiiightmareXD, #2638) there is already a stabilized Rust type that encapsulates HANDLE and calls CloseHandle() on drop(): https://doc.rust-lang.org/stable/std/os/windows/io/struct.OwnedHandle.html

While it currently seems to be impossible for windows-rs to know when ownership transfer occurs (see above: the metadata doesn't describe this), perhaps it could at least implement some of these std traits such as AsHandle and AsRawHandle, and maybe an fn into_owned_handle(self) -> OwnedHandle?

@riverar
Copy link
Collaborator

riverar commented Sep 6, 2023

Don't think we have any plans to adopt those std traits/structs. Opinion: At a glance, the handle work there looks very brittle and naive.

@MarijnS95
Copy link
Contributor

@riverar perhaps it is worth to voice your concerns (if you haven't already done so) upstream?

@kennykerr
Copy link
Collaborator

See #3013 for a solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

5 participants