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

Add support for freeing handles automatically #3013

Merged
merged 4 commits into from
May 1, 2024
Merged

Add support for freeing handles automatically #3013

merged 4 commits into from
May 1, 2024

Conversation

kennykerr
Copy link
Collaborator

@kennykerr kennykerr commented Apr 30, 2024

The Win32 metadata includes information about how to free certain kinds of handles. Since ownership is often unknown or contextual, this is not used to implement the standard Drop trait. Instead, a new Free trait provided by the windows-core crate is implemented for all handles with appropriate metadata. The free function can be used to free the handle manually, but an Owned<T> wrapper is provided to call free automatically when the owned handle is dropped.

This is supported by nearly 150 different handle types - not just HANDLE - including various crypto handle types, registry, GDI and USER32 types, and many more.

Workarounds are provided for the following Win32 metadata bugs:

Here's an example.

Before:

let event = CreateEventW(None, true, false, None)?;
SetEvent(event)?;
WaitForSingleObject(event, 0);
_ = CloseHandle(event);

After:

let event = Owned::new(CreateEventW(None, true, false, None)?);
SetEvent(*event)?;
WaitForSingleObject(*event, 0);

The BCrypt test provides a more comprehensive example: e386c51

Previously, the test was just leaking. You can see how Owned supports all kinds of BCrypt handle types through type inference.

Related: #2638 #2468 #2222

@kennykerr kennykerr merged commit 52352c0 into master May 1, 2024
71 checks passed
@kennykerr kennykerr deleted the free branch May 1, 2024 11:34
@MarijnS95
Copy link
Contributor

MarijnS95 commented May 1, 2024

This looks like really nice stuff! Is the metadata enriched to know when functions like CreateEventW() return an owned handle so that they could possibly return Owned<> straight away in the future, forcing the user to handle automatic lifetime management?

@kennykerr
Copy link
Collaborator Author

This looks like really nice stuff! Is the metadata enriched to know when functions like CreateEventW() return an owned handle so that they could possibly return Owned<> straight away in the future, forcing the user to handle automatic lifetime management?

This isn't very clear today. Hopefully in future...

mati865 pushed a commit to mati865/windows-rs that referenced this pull request Jun 15, 2024
@kennykerr kennykerr mentioned this pull request Jun 19, 2024
zadjii-msft pushed a commit to microsoft/sudo that referenced this pull request Jul 1, 2024
As it happens, another update to the `windows` family of crates is available:
https://github.com/microsoft/windows-rs/releases/tag/0.57.0

In particular, this one includes a new feature to support freeing handles automatically:
microsoft/windows-rs#3013

This avoids the need for the `OwnedHandle` helper type in Sudo.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants