From aff6b930a1ad3650a3703dee5d88b1943fb7d99a Mon Sep 17 00:00:00 2001 From: Chun-wei Fan Date: Mon, 11 Nov 2024 17:42:53 +0800 Subject: [PATCH 1/5] gwin32: Add g_win32_clear_com() This is quite similar in concept to what g_clear_object() does, except that is meant to deal with Windows COM objects. Note that there is a separate C++ version available for this as there are COM interfaces that are available in C++ only, such as DirectWrite that is shipped with the Windows SDK (albeit a C interface is provided with the mingw-w64 toolchain. This will call `->Release()` on the non-NULL COM object referenced by its pointer and sets the COM object to NULL; if the pointer refers to a NULL COM object, this is a no-op. --- glib/gwin32.h | 57 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/glib/gwin32.h b/glib/gwin32.h index e38a7f90a..63460f2f1 100644 --- a/glib/gwin32.h +++ b/glib/gwin32.h @@ -43,6 +43,11 @@ G_BEGIN_DECLS #ifdef G_OS_WIN32 +/* needed include for C++ builds; including this in C mode will cause havoc of type conflicts */ +#ifdef __cplusplus +#include +#endif + /* * To get prototypes for the following POSIXish functions, you have to * include the indicated non-POSIX headers. The functions are defined @@ -135,8 +140,60 @@ gboolean g_win32_check_windows_version (const gint major, const gint spver, const GWin32OSType os_type); +/** + * g_win32_clear_com: + * @com_obj: (not optional) (nullable): Pointer to COM object pointer to release and clear + * + * Releases the referenced COM object, and clears its pointer to `NULL`. + * + * The @com_obj pointer must not be `NULL`. + * + * If @com_obj references a `NULL` COM object, this function is a no-op. + * + * This is equivalent to `g_clear_object()` for dealing with + * Windows COM objects. + * + * Since: 2.84 + */ + +#ifndef __cplusplus + +#define g_win32_clear_com(com_obj) \ +G_STMT_START {\ + IUnknown **unknown_com_obj = (IUnknown **)(com_obj); \ + \ + if (*unknown_com_obj) \ + { \ + (*unknown_com_obj)->lpVtbl->Release (*unknown_com_obj); \ + *unknown_com_obj = NULL; \ + } \ +} G_STMT_END \ +GLIB_AVAILABLE_MACRO_IN_2_84 + +#endif + G_END_DECLS +#ifdef __cplusplus +/* + * There are COM objects that only have C++-style definitions, such as DirectWrite + * from the Windows SDK (albeit a C interface is provided for the mingw-w64 toolchain), + * so we need to have a C++ version for this + */ +template +inline void +g_win32_clear_com (com_interface **com_obj) +{ + IUnknown **unknown_com_obj = reinterpret_cast(com_obj); + + if (*unknown_com_obj != NULL) + { + (*unknown_com_obj)->Release (); + *unknown_com_obj = NULL; + } +} +#endif + #endif /* G_PLATFORM_WIN32 */ #endif /* __G_WIN32_H__ */ From 564233160305e13e41f07b3e9a8e57bb18028ce7 Mon Sep 17 00:00:00 2001 From: Chun-wei Fan Date: Mon, 9 Dec 2024 13:17:14 +0800 Subject: [PATCH 2/5] GLib tests: Add test for g_win32_clear_com() This mimicks the test for g_clear_object() in gobject/tests/reference.c. We use the system's wincodec library for our sample here. --- glib/tests/meson.build | 4 +++- glib/tests/win32.c | 25 +++++++++++++++++++++++++ meson.build | 3 +++ 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/glib/tests/meson.build b/glib/tests/meson.build index 10886f698..03b7a5ca8 100644 --- a/glib/tests/meson.build +++ b/glib/tests/meson.build @@ -246,7 +246,9 @@ if host_machine.system() == 'windows' } endif glib_tests += { - 'win32' : {}, + 'win32' : { + 'dependencies' : [wincodecs], + }, } else glib_tests += { diff --git a/glib/tests/win32.c b/glib/tests/win32.c index 18772c131..cea548707 100644 --- a/glib/tests/win32.c +++ b/glib/tests/win32.c @@ -28,6 +28,9 @@ #include #include +#define COBJMACROS +#include + static char *argv0 = NULL; #include "../gwin32-private.c" @@ -154,6 +157,27 @@ veh_debugger (int argc, char *argv[]) g_fprintf (stderr, "Debugger invoked, attaching to %lu and signalling %" G_GUINTPTR_FORMAT, pid, event); } +static void +test_clear_com (void) +{ + IWICImagingFactory *o = NULL; + IWICImagingFactory *tmp; + + CoInitialize (NULL); + g_win32_clear_com (&o); + g_assert_null (o); + g_assert_true (SUCCEEDED (CoCreateInstance (&CLSID_WICImagingFactory, NULL, CLSCTX_INPROC_SERVER, &IID_IWICImagingFactory, (void **)&tmp))); + g_assert_nonnull (tmp); + IWICImagingFactory_QueryInterface (tmp, &IID_IWICImagingFactory, (void **)&o); /* IWICImagingFactory_QueryInterface increments the refcount */ + g_assert_nonnull (o); + g_assert_cmpint (IWICImagingFactory_AddRef (o), ==, 3); /* refcount incremented again */ + g_win32_clear_com (&o); + g_assert_null (o); + IWICImagingFactory_Release (tmp); + + CoUninitialize (); +} + int main (int argc, char *argv[]) @@ -177,6 +201,7 @@ main (int argc, g_test_add_func ("/win32/subprocess/debuggee", test_veh_debuggee); g_test_add_func ("/win32/subprocess/access_violation", test_access_violation); g_test_add_func ("/win32/subprocess/illegal_instruction", test_illegal_instruction); + g_test_add_func ("/win32/com/clear", test_clear_com); return g_test_run(); } diff --git a/meson.build b/meson.build index 782636da7..c8a2e3760 100644 --- a/meson.build +++ b/meson.build @@ -2338,8 +2338,11 @@ endif if host_system == 'windows' winsock2 = cc.find_library('ws2_32') + # Used for g_win32_clear_com() tests... + wincodecs = cc.find_library('windowscodecs') else winsock2 = not_found + wincodecs = not_found endif selinux_dep = [] From a3a95c2a0b529a5e736d2c2b78c92095d5da3985 Mon Sep 17 00:00:00 2001 From: Chun-wei Fan Date: Mon, 9 Dec 2024 18:41:53 +0800 Subject: [PATCH 3/5] glib/tests: Add C++ test for g_win32_clear_com() This tests the C++ version of g_win32_clear_com() as there are some COM interfaces that are only available in C++, such as DirectWrite from the headers shipped with the Windows SDK. This mimicks the test for the same function in glib/tests/win32.c but done in a C++ fashion. --- glib/tests/cxx.cpp | 30 ++++++++++++++++++++++++++++++ glib/tests/meson.build | 2 ++ 2 files changed, 32 insertions(+) diff --git a/glib/tests/cxx.cpp b/glib/tests/cxx.cpp index 5198621dc..b3a5d684f 100644 --- a/glib/tests/cxx.cpp +++ b/glib/tests/cxx.cpp @@ -19,6 +19,10 @@ #include +#ifdef G_OS_WIN32 +#include +#endif + #if !defined (G_CXX_STD_VERSION) || !defined (G_CXX_STD_CHECK_VERSION) #error G_CXX_STD_VERSION is not defined #endif @@ -532,6 +536,29 @@ test_string_free (void) g_free (data); } +#ifdef G_OS_WIN32 +static void +test_clear_com (void) +{ + IWICImagingFactory *tmp; + IWICImagingFactory *o = NULL; + + CoInitialize (NULL); + g_win32_clear_com (&o); + g_assert_null (o); + g_assert_true (SUCCEEDED (CoCreateInstance (CLSID_WICImagingFactory, NULL, CLSCTX_INPROC_SERVER, IID_IWICImagingFactory, (void **)&tmp))); + g_assert_nonnull (tmp); + tmp->QueryInterface (IID_IWICImagingFactory, (void **)&o); /* IWICImagingFactory::QueryInterface increments the refcount */ + g_assert_nonnull (o); + g_assert_cmpint (o->AddRef (), ==, 3); /* refcount incremented again */ + g_win32_clear_com (&o); + g_assert_null (o); + tmp->Release (); + + CoUninitialize (); +} +#endif + int main (int argc, char *argv[]) { @@ -566,5 +593,8 @@ main (int argc, char *argv[]) g_test_add_func ("/C++/string-append", test_string_append); g_test_add_func ("/C++/string-free", test_string_free); +#ifdef G_OS_WIN32 + g_test_add_func ("/C++/test_clear_com", test_clear_com); +#endif return g_test_run (); } diff --git a/glib/tests/meson.build b/glib/tests/meson.build index 03b7a5ca8..4b77cd93f 100644 --- a/glib/tests/meson.build +++ b/glib/tests/meson.build @@ -209,6 +209,7 @@ if have_cxx 'cxx' : { 'source' : ['cxx.cpp'], 'suite': ['cpp'], + 'dependencies' : [wincodecs], } } @@ -218,6 +219,7 @@ if have_cxx 'source' : ['cxx.cpp'], 'suite' : ['cpp'], 'cpp_args' : [arg, '-D_G_EXPECTED_CXX_STANDARD="@0@"'.format(std)], + 'dependencies' : [wincodecs], }, } endforeach From 4ab7977a4d3793a6ab1addc77eac4fc87f84f7ac Mon Sep 17 00:00:00 2001 From: Chun-wei Fan Date: Fri, 13 Dec 2024 11:03:32 +0800 Subject: [PATCH 4/5] gwin32.h: Make g_win32_clear_com() static inline ...for C++ implementations, as suggested by Luca Bacci. Make things safer in terms of avoiding ODR violations[1]. [1]: https://gitlab.gnome.org/GNOME/glib/-/merge_requests/4392#note_2296862 --- glib/gwin32.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/glib/gwin32.h b/glib/gwin32.h index 63460f2f1..182d23fcd 100644 --- a/glib/gwin32.h +++ b/glib/gwin32.h @@ -181,7 +181,7 @@ G_END_DECLS * so we need to have a C++ version for this */ template -inline void +static inline void g_win32_clear_com (com_interface **com_obj) { IUnknown **unknown_com_obj = reinterpret_cast(com_obj); From 2af6baad08dd028352116bb5ad775af7d2242288 Mon Sep 17 00:00:00 2001 From: Chun-wei Fan Date: Fri, 13 Dec 2024 11:09:03 +0800 Subject: [PATCH 5/5] tests: Add more tests for COM refcounts As suggested by Luca Bacci, so that we keep track of things more clearly in terms of COM. --- glib/tests/cxx.cpp | 10 ++++++---- glib/tests/win32.c | 10 ++++++---- 2 files changed, 12 insertions(+), 8 deletions(-) diff --git a/glib/tests/cxx.cpp b/glib/tests/cxx.cpp index b3a5d684f..4b4dd5800 100644 --- a/glib/tests/cxx.cpp +++ b/glib/tests/cxx.cpp @@ -548,12 +548,14 @@ test_clear_com (void) g_assert_null (o); g_assert_true (SUCCEEDED (CoCreateInstance (CLSID_WICImagingFactory, NULL, CLSCTX_INPROC_SERVER, IID_IWICImagingFactory, (void **)&tmp))); g_assert_nonnull (tmp); - tmp->QueryInterface (IID_IWICImagingFactory, (void **)&o); /* IWICImagingFactory::QueryInterface increments the refcount */ + tmp->QueryInterface (IID_IWICImagingFactory, (void **)&o); /* IWICImagingFactory::QueryInterface increments tmp's refcount */ g_assert_nonnull (o); - g_assert_cmpint (o->AddRef (), ==, 3); /* refcount incremented again */ - g_win32_clear_com (&o); + g_assert_cmpint (tmp->AddRef (), ==, 3); /* tmp's refcount incremented again */ + g_win32_clear_com (&o); /* tmp's refcount gets decremented */ g_assert_null (o); - tmp->Release (); + g_assert_cmpint (tmp->Release (), ==, 1); /* tmp's refcount gets decremented, again */ + g_win32_clear_com (&tmp); + g_assert_null (tmp); CoUninitialize (); } diff --git a/glib/tests/win32.c b/glib/tests/win32.c index cea548707..5e10f0338 100644 --- a/glib/tests/win32.c +++ b/glib/tests/win32.c @@ -168,12 +168,14 @@ test_clear_com (void) g_assert_null (o); g_assert_true (SUCCEEDED (CoCreateInstance (&CLSID_WICImagingFactory, NULL, CLSCTX_INPROC_SERVER, &IID_IWICImagingFactory, (void **)&tmp))); g_assert_nonnull (tmp); - IWICImagingFactory_QueryInterface (tmp, &IID_IWICImagingFactory, (void **)&o); /* IWICImagingFactory_QueryInterface increments the refcount */ + IWICImagingFactory_QueryInterface (tmp, &IID_IWICImagingFactory, (void **)&o); /* IWICImagingFactory_QueryInterface increments tmp's refcount */ g_assert_nonnull (o); - g_assert_cmpint (IWICImagingFactory_AddRef (o), ==, 3); /* refcount incremented again */ - g_win32_clear_com (&o); + g_assert_cmpint (IWICImagingFactory_AddRef (tmp), ==, 3); /* tmp's refcount incremented, again */ + g_win32_clear_com (&o); /* tmp's refcount decrements */ g_assert_null (o); - IWICImagingFactory_Release (tmp); + g_assert_cmpint (IWICImagingFactory_Release (tmp), ==, 1); /* tmp's refcount decrements, again */ + g_win32_clear_com (&tmp); + g_assert_null (tmp); CoUninitialize (); }