From f6a38049142a1860106af26d1befdbf42aff7982 Mon Sep 17 00:00:00 2001 From: junkmd Date: Wed, 4 Nov 2026 09:05:30 +0900 Subject: [PATCH 1/9] refactor: Move `SafeArrayGetRecordInfo` setup to module level. - Define `_SafeArrayGetRecordInfo` at module level using c_void_p in argtypes instead of `POINTER(IRecordInfo)`, eliminating the need for local import. - Use byref for result parameter following surrounding code style. - Add docstring and type ignore comment for return value. --- comtypes/_safearray.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/comtypes/_safearray.py b/comtypes/_safearray.py index 01c3195c8..aede0d0f1 100644 --- a/comtypes/_safearray.py +++ b/comtypes/_safearray.py @@ -77,17 +77,20 @@ def SafeArrayGetVartype(pa): _SafeArrayGetVartype(pa, result) return result.value + +_SafeArrayGetRecordInfo = _oleaut32.SafeArrayGetRecordInfo +_SafeArrayGetRecordInfo.restype = HRESULT +_SafeArrayGetRecordInfo.argtypes = [POINTER(SAFEARRAY), POINTER(c_void_p)] + + def SafeArrayGetRecordInfo(pa): # Defined here to avoid a circular import from comtypes.typeinfo import IRecordInfo - _SafeArrayGetRecordInfo = _oleaut32.SafeArrayGetRecordInfo - _SafeArrayGetRecordInfo.restype = HRESULT - _SafeArrayGetRecordInfo.argtypes = [POINTER(SAFEARRAY), POINTER(POINTER(IRecordInfo))] - result = POINTER(IRecordInfo)() _SafeArrayGetRecordInfo(pa, byref(result)) - return result.value + return result # type: ignore + SafeArrayGetElement = _oleaut32.SafeArrayGetElement SafeArrayGetElement.restype = HRESULT From 4b74403f8b28e43511a7e8947d565bf61bebbc4d Mon Sep 17 00:00:00 2001 From: junkmd Date: Wed, 4 Nov 2026 09:05:30 +0900 Subject: [PATCH 2/9] refactor: Update type hints for `SafeArrayGetRecordInfo` and import `IRecordInfo`. --- comtypes/_safearray.py | 11 ++++++++++- comtypes/hints.pyi | 4 +++- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/comtypes/_safearray.py b/comtypes/_safearray.py index aede0d0f1..b7bc12b59 100644 --- a/comtypes/_safearray.py +++ b/comtypes/_safearray.py @@ -2,9 +2,16 @@ from ctypes import POINTER, Structure, WinDLL, byref, c_uint, c_ushort, c_void_p from ctypes.wintypes import DWORD, LONG, UINT, USHORT +from typing import TYPE_CHECKING +from typing import Union as _UnionT from comtypes import GUID, HRESULT +if TYPE_CHECKING: + from ctypes import _Pointer + + from comtypes import hints # type: ignore + ################################################################ # if __debug__: # from ctypeslib.dynamic_module import include @@ -83,7 +90,9 @@ def SafeArrayGetVartype(pa): _SafeArrayGetRecordInfo.argtypes = [POINTER(SAFEARRAY), POINTER(c_void_p)] -def SafeArrayGetRecordInfo(pa): +def SafeArrayGetRecordInfo( + pa: "_UnionT[hints.LP_SAFEARRAY, _Pointer[tagSAFEARRAY]]", +) -> "hints.IRecordInfo": # Defined here to avoid a circular import from comtypes.typeinfo import IRecordInfo diff --git a/comtypes/hints.pyi b/comtypes/hints.pyi index 00c994d63..7815740a1 100644 --- a/comtypes/hints.pyi +++ b/comtypes/hints.pyi @@ -36,7 +36,9 @@ from comtypes import IUnknown as IUnknown, COMObject as COMObject, GUID as GUID from comtypes.automation import IDispatch as IDispatch, VARIANT as VARIANT from comtypes.server import IClassFactory as IClassFactory from comtypes.server import localserver as localserver -from comtypes.typeinfo import ITypeInfo as ITypeInfo, ITypeLib as ITypeLib +from comtypes.typeinfo import IRecordInfo as IRecordInfo +from comtypes.typeinfo import ITypeInfo as ITypeInfo +from comtypes.typeinfo import ITypeLib as ITypeLib from comtypes._safearray import tagSAFEARRAY as tagSAFEARRAY Incomplete: TypeAlias = Any From 3a2a1c4bb37ee5dc3eb563a528a4aa5a22b8b859 Mon Sep 17 00:00:00 2001 From: junkmd Date: Wed, 4 Nov 2026 09:05:30 +0900 Subject: [PATCH 3/9] test: Skip `test_reverse_records_length_3` due to characterization clash. --- comtypes/test/test_record_safearray_bstr.py | 1 + 1 file changed, 1 insertion(+) diff --git a/comtypes/test/test_record_safearray_bstr.py b/comtypes/test/test_record_safearray_bstr.py index cc4448b3f..b66312d6c 100644 --- a/comtypes/test/test_record_safearray_bstr.py +++ b/comtypes/test/test_record_safearray_bstr.py @@ -387,6 +387,7 @@ def test_first_record_empty_safearray_by_create(self): del records gc.collect() + @unittest.skip("CLASH! CHARACTERIZATION(#63)") def test_reverse_records_length_3(self): ifc = _create_server(self.interface) From 77e33e81fbebb59dd5cda0993e8bcf0749079303 Mon Sep 17 00:00:00 2001 From: junkmd Date: Wed, 4 Nov 2026 09:05:30 +0900 Subject: [PATCH 4/9] test: Convert record `SAFEARRAY` characterization tests to normal-path. Since commit 0681ffe, unpacking VT_ARRAYs containing VT_RECORDs no longer raises `KeyError('36')`. Update the characterization tests for `WrapBstrRecord`, `WrapValueRecord`, and `ReverseValueRecords` to verify the expected return values on the normal path instead of expecting a `KeyError`. --- comtypes/test/test_record_safearray_bstr.py | 14 +++++----- comtypes/test/test_record_safearray_value.py | 27 ++++++++------------ 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/comtypes/test/test_record_safearray_bstr.py b/comtypes/test/test_record_safearray_bstr.py index b66312d6c..4aafb1af2 100644 --- a/comtypes/test/test_record_safearray_bstr.py +++ b/comtypes/test/test_record_safearray_bstr.py @@ -297,14 +297,12 @@ def test_wrap_record(self): ifc = _create_server(self.interface) record = _create_record(id=500, seq=0, text="WrapBstr-500-0", checksum=50708) - with self.assertRaises( - KeyError, - msg="CHARACTERIZATION(#63): " - "If https://github.com/enthought/comtypes/issues/63 is fixed, " - "this failure is expected; change it to a normal-path test.", - ) as cm: - ifc.WrapBstrRecord(record) - self.assertEqual(str(cm.exception), "36") + result = ifc.WrapBstrRecord(record) + self.assertEqual(len(result), 1) + self.assertEqual(result[0].id, 500) + self.assertEqual(result[0].seq, 0) + self.assertEqual(str(result[0].text), "WrapBstr-500-0") + self.assertEqual(result[0].checksum, 50708) del record gc.collect() diff --git a/comtypes/test/test_record_safearray_value.py b/comtypes/test/test_record_safearray_value.py index 74259322b..f2423c8ed 100644 --- a/comtypes/test/test_record_safearray_value.py +++ b/comtypes/test/test_record_safearray_value.py @@ -292,14 +292,11 @@ def test_wrap_record(self): ifc = _create_server(self.interface) record = _create_record(id=300, seq=0, checksum=30042) - with self.assertRaises( - KeyError, - msg="CHARACTERIZATION(#63): " - "If https://github.com/enthought/comtypes/issues/63 is fixed, " - "this failure is expected; change it to a normal-path test.", - ) as cm: - ifc.WrapValueRecord(record) - self.assertEqual(str(cm.exception), "36") + result = ifc.WrapValueRecord(record) + self.assertEqual(len(result), 1) + self.assertEqual(result[0].id, 300) + self.assertEqual(result[0].seq, 0) + self.assertEqual(result[0].checksum, 30042) del record gc.collect() @@ -391,14 +388,12 @@ def test_reverse_records_length_3(self): ] lp = _get_value_record_sa_type().from_param(records) - with self.assertRaises( - KeyError, - msg="CHARACTERIZATION(#63): " - "If https://github.com/enthought/comtypes/issues/63 is fixed, " - "this failure is expected; change it to a normal-path test.", - ) as cm: - ifc.ReverseValueRecords(lp) - self.assertEqual(str(cm.exception), "36") + result = ifc.ReverseValueRecords(lp) + self.assertEqual(len(result), 3) + self.assertEqual( + [(r.id, r.seq, r.checksum) for r in result], + [(s.id, s.seq, s.checksum) for s in reversed(records)], + ) del records gc.collect() From f53c236aaefb5c5d397109fea75428fa3e7ad64e Mon Sep 17 00:00:00 2001 From: junkmd Date: Wed, 4 Nov 2026 09:05:30 +0900 Subject: [PATCH 5/9] test: Add test to check `ReverseBstrRecords` method does not crash. This test uses `faulthandler` and `multiprocessing` to run the test in a separate process. --- comtypes/test/test_record_safearray_bstr.py | 62 ++++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) diff --git a/comtypes/test/test_record_safearray_bstr.py b/comtypes/test/test_record_safearray_bstr.py index 4aafb1af2..5c15d39f2 100644 --- a/comtypes/test/test_record_safearray_bstr.py +++ b/comtypes/test/test_record_safearray_bstr.py @@ -4,10 +4,16 @@ strings) between Python and a COM server. """ +import faulthandler import gc +import io +import multiprocessing +import tempfile import unittest from _ctypes import COMError -from typing import TYPE_CHECKING, ClassVar, TypeVar +from collections.abc import Callable +from pathlib import Path +from typing import TYPE_CHECKING, Any, ClassVar, Optional, TypeVar from typing import Union as _UnionT from comtypes import CLSCTX_LOCAL_SERVER, IUnknown, _safearray @@ -409,5 +415,59 @@ def test_reverse_records_length_3(self): gc.collect() +def enable_fault_log(tmpfile: Path) -> io.TextIOWrapper: + """Send faulthandler output to `tmpfile` until the process exits.""" + f = tmpfile.open(mode="w", encoding="utf-8") + # We keep `faulthandler` enabled and the file open until the process + # exits so that crash information can be captured. + # It is only enabled in the child process and does not affect the main + # process, so there is no need to worry about forgetting to close and + # disable. + faulthandler.enable(file=f) + return f + + +def run_in_mp(fn: Callable[[Path], Any]) -> tuple[Optional[int], str]: + """Run a function in a separate process and return exit code and output. + + Using spawn isolates the process so crashes like segfaults do not affect + the main test process. + """ + with tempfile.TemporaryDirectory() as td: + tmpdir = Path(td) + tmpfile = tmpdir / "fault.txt" + ctx = multiprocessing.get_context("spawn") + process = ctx.Process(target=fn, args=(tmpfile,)) + process.start() + process.join() + output = tmpfile.read_text() + return process.exitcode, output + + +def call_reverse_bstr_records(tmpfile: Path) -> None: + enable_fault_log(tmpfile) + ifc = _create_server(ComtypesCppTestSrvLib.IDispRecordSafearrayTest) + + records = [ + _create_record(id=401, seq=0, text="RecSafeArray-401-0", checksum=40808), + _create_record(id=402, seq=1, text="RecSafeArray-402-1", checksum=40918), + _create_record(id=403, seq=2, text="RecSafeArray-403-2", checksum=41028), + ] + lp = _get_bstr_record_sa_type().from_param(records) + + result = ifc.ReverseBstrRecords(lp) + + del records + del result + del lp + gc.collect() + + +class TestOwnership(unittest.TestCase): + def test_bstr_record_safearray_ownership(self): + exitcode, output = run_in_mp(call_reverse_bstr_records) + self.assertEqual(exitcode, 0, msg=f"Process crashed with output: {output}") + + if __name__ == "__main__": unittest.main() From a55aff6315ca23b3a5824bff84146688862829e1 Mon Sep 17 00:00:00 2001 From: junkmd Date: Wed, 4 Nov 2026 09:05:30 +0900 Subject: [PATCH 6/9] test: Convert `call_reverse_bstr_records` to characterization test. The test `call_reverse_bstr_records` was an existing test. This change converts it to a characterization test. It captures and verifies the crash without breaking the whole test suite. The test probably crashes because of a double-free bug. Root cause (hypothesis): - The `VARIANT` has a `SAFEARRAY`. This `SAFEARRAY` probably shares the same `BSTR` pointer as `lp`. - After the COM call returns, `VariantClear` runs. `VariantClear` probably frees that `BSTR`. - Then Python runs `del lp`. This probably tries to free the same `BSTR` again. - This probably causes a double-free and heap corruption. --- comtypes/test/test_record_safearray_bstr.py | 27 ++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/comtypes/test/test_record_safearray_bstr.py b/comtypes/test/test_record_safearray_bstr.py index 5c15d39f2..c1898ade4 100644 --- a/comtypes/test/test_record_safearray_bstr.py +++ b/comtypes/test/test_record_safearray_bstr.py @@ -8,6 +8,7 @@ import gc import io import multiprocessing +import re import tempfile import unittest from _ctypes import COMError @@ -463,10 +464,34 @@ def call_reverse_bstr_records(tmpfile: Path) -> None: gc.collect() +# Windows NTSTATUS values (unsigned 32-bit). +# Python's process.exitcode may return these as unsigned integers +# (e.g., 3221226356 = 0xC0000374) depending on the platform/Python version. +# We mask with 0xFFFFFFFF to ensure consistent comparison. +STATUS_HEAP_CORRUPTION = 0xC0000374 +STATUS_ACCESS_VIOLATION = 0xC0000005 +STATUS_STACK_BUFFER_OVERRUN = 0xC0000409 + + class TestOwnership(unittest.TestCase): def test_bstr_record_safearray_ownership(self): exitcode, output = run_in_mp(call_reverse_bstr_records) - self.assertEqual(exitcode, 0, msg=f"Process crashed with output: {output}") + self.assertIsNotNone(exitcode) + # Python's process.exitcode on Windows may return the exit code as + # either a signed or unsigned 32-bit integer. Masking with 0xFFFFFFFF + # normalizes it to the unsigned 32-bit representation for comparison + # with the NTSTATUS constants defined above. + self.assertEqual( + exitcode & 0xFFFFFFFF, # type: ignore + STATUS_HEAP_CORRUPTION, + msg="CHARACTERIZATION(#63): " + "If https://github.com/enthought/comtypes/issues/63 is fixed, " + "this failure is expected; change it to a normal-path test.", + ) + ptn = re.compile( + r'test_record_safearray_bstr\.py", line \d+ in call_reverse_bstr_records' + ) + self.assertTrue(bool(ptn.search(output))) if __name__ == "__main__": From 068d1a7520a50b888ad11ce7bf531550eb0d2963 Mon Sep 17 00:00:00 2001 From: junkmd Date: Wed, 4 Nov 2026 22:09:32 +0900 Subject: [PATCH 7/9] test: Refactor tests for `ReverseBstrRecords` handling. Updated to handle different Python versions. --- comtypes/test/test_record_safearray_bstr.py | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/comtypes/test/test_record_safearray_bstr.py b/comtypes/test/test_record_safearray_bstr.py index c1898ade4..eca03e35c 100644 --- a/comtypes/test/test_record_safearray_bstr.py +++ b/comtypes/test/test_record_safearray_bstr.py @@ -9,6 +9,7 @@ import io import multiprocessing import re +import sys import tempfile import unittest from _ctypes import COMError @@ -392,7 +393,7 @@ def test_first_record_empty_safearray_by_create(self): del records gc.collect() - @unittest.skip("CLASH! CHARACTERIZATION(#63)") + @unittest.skipIf(sys.version_info >= (3, 14), "CRASH! CHARACTERIZATION(#63)") def test_reverse_records_length_3(self): ifc = _create_server(self.interface) @@ -403,14 +404,12 @@ def test_reverse_records_length_3(self): ] lp = _get_bstr_record_sa_type().from_param(records) - with self.assertRaises( - KeyError, - msg="CHARACTERIZATION(#63): " - "If https://github.com/enthought/comtypes/issues/63 is fixed, " - "this failure is expected; change it to a normal-path test.", - ) as cm: - ifc.ReverseBstrRecords(lp) - self.assertEqual(str(cm.exception), "36") + result = ifc.ReverseBstrRecords(lp) + self.assertEqual(len(result), 3) + self.assertEqual( + [(r.id, r.seq, r.text, r.checksum) for r in result], + [(s.id, s.seq, s.text, s.checksum) for s in reversed(records)], + ) del records gc.collect() @@ -474,6 +473,7 @@ def call_reverse_bstr_records(tmpfile: Path) -> None: class TestOwnership(unittest.TestCase): + @unittest.skipIf(sys.version_info < (3, 14), "NOT CRASH! CHARACTERIZATION(#63)") def test_bstr_record_safearray_ownership(self): exitcode, output = run_in_mp(call_reverse_bstr_records) self.assertIsNotNone(exitcode) From 893f402159165d28cccecff8ffdee8e137a610a2 Mon Sep 17 00:00:00 2001 From: junkmd Date: Wed, 4 Nov 2026 22:09:32 +0900 Subject: [PATCH 8/9] fix: Simplify redundant type annotations. Since `LP_SAFEARRAY` is also `_Pointer[tagSAFEARRAY]`, `_Pointer[tagSAFEARRAY] | LP_SAFEARRAY` was redundant. --- comtypes/_safearray.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/comtypes/_safearray.py b/comtypes/_safearray.py index b7bc12b59..08e50b444 100644 --- a/comtypes/_safearray.py +++ b/comtypes/_safearray.py @@ -3,7 +3,6 @@ from ctypes import POINTER, Structure, WinDLL, byref, c_uint, c_ushort, c_void_p from ctypes.wintypes import DWORD, LONG, UINT, USHORT from typing import TYPE_CHECKING -from typing import Union as _UnionT from comtypes import GUID, HRESULT @@ -90,9 +89,7 @@ def SafeArrayGetVartype(pa): _SafeArrayGetRecordInfo.argtypes = [POINTER(SAFEARRAY), POINTER(c_void_p)] -def SafeArrayGetRecordInfo( - pa: "_UnionT[hints.LP_SAFEARRAY, _Pointer[tagSAFEARRAY]]", -) -> "hints.IRecordInfo": +def SafeArrayGetRecordInfo(pa: "_Pointer[tagSAFEARRAY]") -> "hints.IRecordInfo": # Defined here to avoid a circular import from comtypes.typeinfo import IRecordInfo From f3b4180b1b9ac48b6bc2d76a692c2ae58d1212cc Mon Sep 17 00:00:00 2001 From: junkmd Date: Wed, 4 Nov 2026 22:52:43 +0900 Subject: [PATCH 9/9] test: Skip `TestOwnership` if COM server import fails. --- comtypes/test/test_record_safearray_bstr.py | 1 + 1 file changed, 1 insertion(+) diff --git a/comtypes/test/test_record_safearray_bstr.py b/comtypes/test/test_record_safearray_bstr.py index eca03e35c..4d8dff488 100644 --- a/comtypes/test/test_record_safearray_bstr.py +++ b/comtypes/test/test_record_safearray_bstr.py @@ -472,6 +472,7 @@ def call_reverse_bstr_records(tmpfile: Path) -> None: STATUS_STACK_BUFFER_OVERRUN = 0xC0000409 +@unittest.skipIf(IMPORT_FAILED, "This depends on the out of process COM-server.") class TestOwnership(unittest.TestCase): @unittest.skipIf(sys.version_info < (3, 14), "NOT CRASH! CHARACTERIZATION(#63)") def test_bstr_record_safearray_ownership(self):