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

Automatically dispose COM and WinRT objects with NativeFinalizer #623

Merged
merged 19 commits into from Dec 23, 2022

Conversation

halildurmus
Copy link
Member

@halildurmus halildurmus commented Dec 17, 2022

Fixes #443

Attaches NativeFinalizer to IUnknown objects so that their pointers can be freed automatically by the NativeFinalizer. Note that freeing their pointers is not enough, we actually need to call release() on objects before freeing their pointers so that the resources on the native side can be freed as well. For example, here is a snippet from windows-rs that implements the Drop trait for IUnknown interface.

In order to do this automatically with the NativeFinalizer, we need to write a small function in C++ and use it as the NativeFinalizer's callback. The function is going to be something like this:

#include <windows.h>

#define EXPORT extern "C" __declspec(dllexport)

struct COMObject
{
	LPVOID lpVtbl;
};

EXPORT void DisposeComObject(COMObject *com_object)
{
	IUnknown *unknown = reinterpret_cast<IUnknown *>(com_object->lpVtbl);
	unknown->Release();
	CoTaskMemFree(com_object);
}

Bundling native code with Dart packages is a huge pain at the moment. Fortunately, there is an upcoming feature to make this process seamless. Once it is released, we should be able to easily use the above function as a callback for the NativeFinalizer and get rid of the calls to release(), and make disposing COM and WinRT objects completely automatic (yay!).

Also made some refactor/cleanup in various places.

@halildurmus halildurmus added enhancement New feature or request winrt com Issues relating to COM support labels Dec 17, 2022
@halildurmus halildurmus added this to In progress in WinRT projection via automation Dec 17, 2022
@halildurmus halildurmus marked this pull request as ready for review December 17, 2022 12:26
Copy link
Contributor

@timsneath timsneath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few comments prior to merging; hopefully they're helpful!

lib/src/com/iunknown.dart Outdated Show resolved Hide resolved
lib/src/com/iunknown.dart Outdated Show resolved Hide resolved
lib/src/com/iunknown.dart Outdated Show resolved Hide resolved
lib/winrt.dart Show resolved Hide resolved
test/winrt_calendar_test.dart Show resolved Hide resolved
Copy link
Contributor

@timsneath timsneath left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great! Let's do this! :) Thank you.

WinRT projection automation moved this from In progress to Reviewer approved Dec 23, 2022
@timsneath timsneath merged commit d5fc339 into main Dec 23, 2022
WinRT projection automation moved this from Reviewer approved to Done Dec 23, 2022
@timsneath timsneath deleted the native-finalizer branch December 23, 2022 18:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
com Issues relating to COM support enhancement New feature or request
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

Memory management for WinRT Classes
2 participants