From f3e7afbd046dae0e0b0731b62b61fd0f557365fc Mon Sep 17 00:00:00 2001 From: Jun Komoda <45822440+junkmd@users.noreply.github.com> Date: Sat, 23 Nov 2024 01:10:30 +0000 Subject: [PATCH 1/5] Make `create_shelllink_persist` top level function. --- .../test_win32_com_foreign_func.py | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/Lib/test/test_ctypes/test_win32_com_foreign_func.py b/Lib/test/test_ctypes/test_win32_com_foreign_func.py index 651c9277d59af9..a4458efa4e5993 100644 --- a/Lib/test/test_ctypes/test_win32_com_foreign_func.py +++ b/Lib/test/test_ctypes/test_win32_com_foreign_func.py @@ -78,6 +78,19 @@ def is_equal_guid(guid1, guid2): ) +def create_shelllink_persist(typ): + ppst = typ() + # https://learn.microsoft.com/en-us/windows/win32/api/combaseapi/nf-combaseapi-cocreateinstance + ole32.CoCreateInstance( + byref(CLSID_ShellLink), + None, + CLSCTX_SERVER, + byref(IID_IPersist), + byref(ppst), + ) + return ppst + + class ForeignFunctionsThatWillCallComMethodsTests(unittest.TestCase): def setUp(self): # https://learn.microsoft.com/en-us/windows/win32/api/combaseapi/nf-combaseapi-coinitializeex @@ -88,19 +101,6 @@ def tearDown(self): ole32.CoUninitialize() gc.collect() - @staticmethod - def create_shelllink_persist(typ): - ppst = typ() - # https://learn.microsoft.com/en-us/windows/win32/api/combaseapi/nf-combaseapi-cocreateinstance - ole32.CoCreateInstance( - byref(CLSID_ShellLink), - None, - CLSCTX_SERVER, - byref(IID_IPersist), - byref(ppst), - ) - return ppst - def test_without_paramflags_and_iid(self): class IUnknown(c_void_p): QueryInterface = proto_query_interface() @@ -110,7 +110,7 @@ class IUnknown(c_void_p): class IPersist(IUnknown): GetClassID = proto_get_class_id() - ppst = self.create_shelllink_persist(IPersist) + ppst = create_shelllink_persist(IPersist) clsid = GUID() hr_getclsid = ppst.GetClassID(byref(clsid)) @@ -142,7 +142,7 @@ class IUnknown(c_void_p): class IPersist(IUnknown): GetClassID = proto_get_class_id(((OUT, "pClassID"),)) - ppst = self.create_shelllink_persist(IPersist) + ppst = create_shelllink_persist(IPersist) clsid = ppst.GetClassID() self.assertEqual(TRUE, is_equal_guid(CLSID_ShellLink, clsid)) @@ -167,7 +167,7 @@ class IUnknown(c_void_p): class IPersist(IUnknown): GetClassID = proto_get_class_id(((OUT, "pClassID"),), IID_IPersist) - ppst = self.create_shelllink_persist(IPersist) + ppst = create_shelllink_persist(IPersist) clsid = ppst.GetClassID() self.assertEqual(TRUE, is_equal_guid(CLSID_ShellLink, clsid)) From c28e7c45265781dc9cad8c941ed4214884ea6717 Mon Sep 17 00:00:00 2001 From: Jun Komoda <45822440+junkmd@users.noreply.github.com> Date: Sat, 23 Nov 2024 01:10:30 +0000 Subject: [PATCH 2/5] Add `CopyComPointerTests`. --- .../test_win32_com_foreign_func.py | 41 ++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) diff --git a/Lib/test/test_ctypes/test_win32_com_foreign_func.py b/Lib/test/test_ctypes/test_win32_com_foreign_func.py index a4458efa4e5993..e3578817846455 100644 --- a/Lib/test/test_ctypes/test_win32_com_foreign_func.py +++ b/Lib/test/test_ctypes/test_win32_com_foreign_func.py @@ -9,7 +9,7 @@ raise unittest.SkipTest("Windows-specific test") -from _ctypes import COMError +from _ctypes import COMError, CopyComPointer from ctypes import HRESULT @@ -184,5 +184,44 @@ class IPersist(IUnknown): self.assertEqual(0, ppst.Release()) +class CopyComPointerTests(unittest.TestCase): + def setUp(self): + ole32.CoInitializeEx(None, COINIT_APARTMENTTHREADED) + + def tearDown(self): + ole32.CoUninitialize() + gc.collect() + + def test_copy_com_pointer(self): + class IUnknown(c_void_p): + QueryInterface = proto_query_interface(None, IID_IUnknown) + AddRef = proto_add_ref() + Release = proto_release() + + class IPersist(IUnknown): + GetClassID = proto_get_class_id(((OUT, "pClassID"),), IID_IPersist) + + src = create_shelllink_persist(IPersist) + dst = IPersist() + + self.assertIsNone(dst.value) + with self.assertRaises(ValueError): + dst.GetClassID() # NULL COM pointer access + + hr = CopyComPointer(src, byref(dst)) + + self.assertEqual(S_OK, hr) + self.assertEqual(dst.value, src.value) + + clsid = dst.GetClassID() + self.assertEqual(TRUE, is_equal_guid(CLSID_ShellLink, clsid)) + + # The refcount of a COM pointer created by `CoCreateInstance` is 1. + # `CopyComPointer` calls `AddRef` internally (thus, +1 to the refcount). + # Here, the refcount is decremented from 2 to 1. + self.assertEqual(1, dst.Release()) + self.assertEqual(0, src.Release()) + + if __name__ == '__main__': unittest.main() From 79f7055fcf221d8eb593493158cc2f64dbe9cfa7 Mon Sep 17 00:00:00 2001 From: Jun Komoda <45822440+junkmd@users.noreply.github.com> Date: Sat, 23 Nov 2024 01:10:30 +0000 Subject: [PATCH 3/5] Add more tests. --- .../test_win32_com_foreign_func.py | 62 +++++++++++++++---- 1 file changed, 49 insertions(+), 13 deletions(-) diff --git a/Lib/test/test_ctypes/test_win32_com_foreign_func.py b/Lib/test/test_ctypes/test_win32_com_foreign_func.py index e3578817846455..3e053465441f69 100644 --- a/Lib/test/test_ctypes/test_win32_com_foreign_func.py +++ b/Lib/test/test_ctypes/test_win32_com_foreign_func.py @@ -188,11 +188,6 @@ class CopyComPointerTests(unittest.TestCase): def setUp(self): ole32.CoInitializeEx(None, COINIT_APARTMENTTHREADED) - def tearDown(self): - ole32.CoUninitialize() - gc.collect() - - def test_copy_com_pointer(self): class IUnknown(c_void_p): QueryInterface = proto_query_interface(None, IID_IUnknown) AddRef = proto_add_ref() @@ -201,26 +196,67 @@ class IUnknown(c_void_p): class IPersist(IUnknown): GetClassID = proto_get_class_id(((OUT, "pClassID"),), IID_IPersist) - src = create_shelllink_persist(IPersist) - dst = IPersist() + self.IUnknown = IUnknown + self.IPersist = IPersist + + def tearDown(self): + ole32.CoUninitialize() + gc.collect() + + def test_both_are_null(self): + src = self.IPersist() + dst = self.IPersist() + + hr = CopyComPointer(src, byref(dst)) + + self.assertEqual(S_OK, hr) + self.assertIsNone(src.value) self.assertIsNone(dst.value) + + def test_both_are_nonnull(self): + src = create_shelllink_persist(self.IPersist) + dst = create_shelllink_persist(self.IPersist) + + self.assertNotEqual(src.value, dst.value) + hr = CopyComPointer(src, byref(dst)) + self.assertEqual(S_OK, hr) + self.assertEqual(src.value, dst.value) + + self.assertEqual(1, src.Release()) + + clsid = dst.GetClassID() + self.assertEqual(TRUE, is_equal_guid(CLSID_ShellLink, clsid)) + + self.assertEqual(0, dst.Release()) + + def test_dest_is_nonnull(self): + src = self.IPersist() + dst = create_shelllink_persist(self.IPersist) + + hr = CopyComPointer(src, byref(dst)) + + self.assertEqual(S_OK, hr) + self.assertIsNone(dst.value) + with self.assertRaises(ValueError): dst.GetClassID() # NULL COM pointer access + def test_src_is_nonnull(self): + src = create_shelllink_persist(self.IPersist) + dst = self.IPersist() + hr = CopyComPointer(src, byref(dst)) self.assertEqual(S_OK, hr) - self.assertEqual(dst.value, src.value) + self.assertEqual(src.value, dst.value) + + self.assertEqual(1, src.Release()) clsid = dst.GetClassID() self.assertEqual(TRUE, is_equal_guid(CLSID_ShellLink, clsid)) - # The refcount of a COM pointer created by `CoCreateInstance` is 1. - # `CopyComPointer` calls `AddRef` internally (thus, +1 to the refcount). - # Here, the refcount is decremented from 2 to 1. - self.assertEqual(1, dst.Release()) - self.assertEqual(0, src.Release()) + self.assertEqual(0, dst.Release()) if __name__ == '__main__': From 638487b32e795d9e406d9db3622152a744b22c26 Mon Sep 17 00:00:00 2001 From: Jun Komoda <45822440+junkmd@users.noreply.github.com> Date: Sun, 24 Nov 2024 00:46:16 +0000 Subject: [PATCH 4/5] Update tests. --- .../test_win32_com_foreign_func.py | 35 +++++++++++++++---- 1 file changed, 29 insertions(+), 6 deletions(-) diff --git a/Lib/test/test_ctypes/test_win32_com_foreign_func.py b/Lib/test/test_ctypes/test_win32_com_foreign_func.py index 3e053465441f69..9d4decf0a65dcf 100644 --- a/Lib/test/test_ctypes/test_win32_com_foreign_func.py +++ b/Lib/test/test_ctypes/test_win32_com_foreign_func.py @@ -214,15 +214,20 @@ def test_both_are_null(self): self.assertIsNone(src.value) self.assertIsNone(dst.value) - def test_both_are_nonnull(self): + def test_src_is_nonnull_and_dest_is_null(self): + # The reference count of the COM pointer created by `CoCreateInstance` + # is initially 1. src = create_shelllink_persist(self.IPersist) - dst = create_shelllink_persist(self.IPersist) + dst = self.IPersist() - self.assertNotEqual(src.value, dst.value) + # `CopyComPointer` calls `AddRef` explicitly in the C implementation. + # The refcount of `src` is incremented from 1 to 2 here. hr = CopyComPointer(src, byref(dst)) + self.assertEqual(S_OK, hr) self.assertEqual(src.value, dst.value) + # This indicates that the refcount was 2 before the `Release` call. self.assertEqual(1, src.Release()) clsid = dst.GetClassID() @@ -230,10 +235,17 @@ def test_both_are_nonnull(self): self.assertEqual(0, dst.Release()) - def test_dest_is_nonnull(self): + def test_src_is_null_and_dest_is_nonnull(self): src = self.IPersist() - dst = create_shelllink_persist(self.IPersist) + dst_orig = create_shelllink_persist(self.IPersist) + dst = self.IPersist() + CopyComPointer(dst_orig, byref(dst)) + dst_orig.Release() # The refcount of `dst_orig` is 1 here. + + clsid = dst.GetClassID() + self.assertEqual(TRUE, is_equal_guid(CLSID_ShellLink, clsid)) + # This does NOT affects the refcount of `dst_orig`. hr = CopyComPointer(src, byref(dst)) self.assertEqual(S_OK, hr) @@ -242,14 +254,24 @@ def test_dest_is_nonnull(self): with self.assertRaises(ValueError): dst.GetClassID() # NULL COM pointer access - def test_src_is_nonnull(self): + # This indicates that the refcount was 1 before the `Release` call. + self.assertEqual(0, dst_orig.Release()) + + def test_both_are_nonnull(self): src = create_shelllink_persist(self.IPersist) + dst_orig = create_shelllink_persist(self.IPersist) dst = self.IPersist() + CopyComPointer(dst_orig, byref(dst)) + dst_orig.Release() + + self.assertEqual(dst.value, dst_orig.value) + self.assertNotEqual(src.value, dst.value) hr = CopyComPointer(src, byref(dst)) self.assertEqual(S_OK, hr) self.assertEqual(src.value, dst.value) + self.assertNotEqual(dst.value, dst_orig.value) self.assertEqual(1, src.Release()) @@ -257,6 +279,7 @@ def test_src_is_nonnull(self): self.assertEqual(TRUE, is_equal_guid(CLSID_ShellLink, clsid)) self.assertEqual(0, dst.Release()) + self.assertEqual(0, dst_orig.Release()) if __name__ == '__main__': From a9d2805e5b3748df6b09d9ce1b77564992e49cdc Mon Sep 17 00:00:00 2001 From: Jun Komoda <45822440+junkmd@users.noreply.github.com> Date: Mon, 25 Nov 2024 13:11:17 +0000 Subject: [PATCH 5/5] Add assertions for `Release`'s return value. --- Lib/test/test_ctypes/test_win32_com_foreign_func.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_ctypes/test_win32_com_foreign_func.py b/Lib/test/test_ctypes/test_win32_com_foreign_func.py index 9d4decf0a65dcf..8d217fc17efa02 100644 --- a/Lib/test/test_ctypes/test_win32_com_foreign_func.py +++ b/Lib/test/test_ctypes/test_win32_com_foreign_func.py @@ -240,7 +240,7 @@ def test_src_is_null_and_dest_is_nonnull(self): dst_orig = create_shelllink_persist(self.IPersist) dst = self.IPersist() CopyComPointer(dst_orig, byref(dst)) - dst_orig.Release() # The refcount of `dst_orig` is 1 here. + self.assertEqual(1, dst_orig.Release()) clsid = dst.GetClassID() self.assertEqual(TRUE, is_equal_guid(CLSID_ShellLink, clsid)) @@ -262,7 +262,7 @@ def test_both_are_nonnull(self): dst_orig = create_shelllink_persist(self.IPersist) dst = self.IPersist() CopyComPointer(dst_orig, byref(dst)) - dst_orig.Release() + self.assertEqual(1, dst_orig.Release()) self.assertEqual(dst.value, dst_orig.value) self.assertNotEqual(src.value, dst.value)