From f7089af7fd4255fb14481a9f1afc3ecff72724a2 Mon Sep 17 00:00:00 2001 From: junkmd Date: Fri, 6 Nov 2026 07:42:15 +0900 Subject: [PATCH 1/8] test: Update `TestOwnership` to check for normal process exit code instead of crash characterization. --- comtypes/test/test_record_safearray_bstr.py | 21 +-------------------- 1 file changed, 1 insertion(+), 20 deletions(-) diff --git a/comtypes/test/test_record_safearray_bstr.py b/comtypes/test/test_record_safearray_bstr.py index 4d8dff488..aa475dce0 100644 --- a/comtypes/test/test_record_safearray_bstr.py +++ b/comtypes/test/test_record_safearray_bstr.py @@ -8,8 +8,6 @@ import gc import io import multiprocessing -import re -import sys import tempfile import unittest from _ctypes import COMError @@ -393,7 +391,6 @@ def test_first_record_empty_safearray_by_create(self): del records gc.collect() - @unittest.skipIf(sys.version_info >= (3, 14), "CRASH! CHARACTERIZATION(#63)") def test_reverse_records_length_3(self): ifc = _create_server(self.interface) @@ -474,25 +471,9 @@ def call_reverse_bstr_records(tmpfile: Path) -> None: @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): exitcode, output = run_in_mp(call_reverse_bstr_records) - 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))) + self.assertEqual(exitcode, 0, msg=f"Process crashed with output: {output}") if __name__ == "__main__": From 50878c647c5a53c08cc5c8b3dbe02b18c8d19df4 Mon Sep 17 00:00:00 2001 From: junkmd Date: Fri, 6 Nov 2026 07:42:15 +0900 Subject: [PATCH 2/8] fix: Change `LP_SAFEARRAY._needsfree` from `ClassVar[bool]` to `bool`. It is an instance attribute, not a class attribute. Each `SAFEARRAY` pointer instance has its own `_needsfree` flag to track memory ownership. --- comtypes/hints.pyi | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/comtypes/hints.pyi b/comtypes/hints.pyi index 7815740a1..ed26ee42d 100644 --- a/comtypes/hints.pyi +++ b/comtypes/hints.pyi @@ -62,7 +62,7 @@ class LP_SAFEARRAY(ctypes._Pointer[tagSAFEARRAY], Generic[_CT]): contents: tagSAFEARRAY _itemtype_: ClassVar[_CT] # type: ignore _vartype_: ClassVar[int] - _needsfree: ClassVar[bool] + _needsfree: bool @overload @classmethod From 3b7ee30af6efaee434a4238609ec05326394ae71 Mon Sep 17 00:00:00 2001 From: junkmd Date: Fri, 6 Nov 2026 07:42:15 +0900 Subject: [PATCH 3/8] feat: Add `is_struct_type_with_recordinfo` function. Add helper `_is_recordinfo_guids` and type alias `_RIGuids` to check if a type is a `Structure` with valid `_recordinfo_` class variable. --- comtypes/typeinfo.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/comtypes/typeinfo.py b/comtypes/typeinfo.py index 200d7ca68..7f7e1067a 100644 --- a/comtypes/typeinfo.py +++ b/comtypes/typeinfo.py @@ -663,6 +663,40 @@ def GetRecordInfoFromTypeInfo(tinfo: ITypeInfo) -> IRecordInfo: return ri # type: ignore +_RIGuids = tuple[str, int, int, int, str] + + +def _is_recordinfo_guids(obj: Any) -> "hints.TypeIs[_RIGuids]": + if not isinstance(obj, tuple): + return False + if len(obj) != 5: + return False + tlibid, majorver, minver, lcid, tinfoid = obj + if ( + isinstance(tlibid, str) + and isinstance(majorver, int) + and isinstance(minver, int) + and isinstance(lcid, int) + and isinstance(tinfoid, str) + ): + return True + return False + + +# TODO: This should be typed as an intersection of structure and protocol. +def is_struct_type_with_recordinfo(typ: type[Any]) -> bool: + """Return `True` if `typ` is a `Structure` type with valid `_recordinfo_` + class variable. + """ + if not issubclass(typ, ctypes.Structure): + return False + if not hasattr(typ, "_recordinfo_"): + return False + if not _is_recordinfo_guids(typ._recordinfo_): + return False + return True + + def GetRecordInfoFromGuids( rGuidTypeLib: str, verMajor: int, verMinor: int, lcid: int, rGuidTypeInfo: str ) -> IRecordInfo: From 269d3b1a9e15217161752b1b21bce29adbf1faa6 Mon Sep 17 00:00:00 2001 From: junkmd Date: Fri, 6 Nov 2026 07:42:15 +0900 Subject: [PATCH 4/8] feat: Implement `SafeArrayCopy` and enhance `_make_safearray_type` to handle record extraction. --- comtypes/_safearray.py | 11 +++++++++ comtypes/safearray.py | 53 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/comtypes/_safearray.py b/comtypes/_safearray.py index 08e50b444..b5284d154 100644 --- a/comtypes/_safearray.py +++ b/comtypes/_safearray.py @@ -98,6 +98,17 @@ def SafeArrayGetRecordInfo(pa: "_Pointer[tagSAFEARRAY]") -> "hints.IRecordInfo": return result # type: ignore +_SafeArrayCopy = _oleaut32.SafeArrayCopy +_SafeArrayCopy.restype = HRESULT +_SafeArrayCopy.argtypes = [POINTER(SAFEARRAY), POINTER(POINTER(SAFEARRAY))] + + +def SafeArrayCopy(pa: "_Pointer[SAFEARRAY]") -> "hints.LP_SAFEARRAY": + result = POINTER(SAFEARRAY)() + _SafeArrayCopy(pa, byref(result)) + return result # type: ignore + + SafeArrayGetElement = _oleaut32.SafeArrayGetElement SafeArrayGetElement.restype = HRESULT SafeArrayGetElement.argtypes = [POINTER(SAFEARRAY), POINTER(LONG), c_void_p] diff --git a/comtypes/safearray.py b/comtypes/safearray.py index f99ce9d1c..3aa2e019c 100644 --- a/comtypes/safearray.py +++ b/comtypes/safearray.py @@ -1,5 +1,6 @@ import array import threading +from collections.abc import Sequence from ctypes import POINTER, Structure, byref, c_long, cast, memmove, pointer, sizeof from typing import TYPE_CHECKING @@ -299,7 +300,10 @@ def _get_elements_raw(self, num_elements): """Returns a flat list or ndarray containing ALL elements in the safearray.""" from comtypes.automation import VARIANT, tagCY, tagDEC + from comtypes.typeinfo import is_struct_type_with_recordinfo + if is_struct_type_with_recordinfo(self._itemtype_): + return self._get_records(num_elements) # XXX Not sure this is true: # For VT_UNKNOWN and VT_DISPATCH, we should retrieve the # interface iid by SafeArrayGetIID(). @@ -365,6 +369,55 @@ def keep_safearray(v): finally: _safearray.SafeArrayUnaccessData(self) + def _get_records(self, num_elements: int) -> Sequence[Structure]: + """Extract element records as a flat list while avoiding double-free + crashes. + + The source `SAFEARRAY` may be destroyed at any time by COM. + Without proper lifetime management, returned elements would suffer + use-after-free, or the array would double-free when its Python + instance is finalized. + + We avoid this by deep-copying the `SAFEARRAY`. The returned + (copied) element record instances each hold a reference to the + copied array instance, keeping the copied array alive until all + returned element records are gone. + + Once the array and all element records are GC'd in Python, the + copied array is automatically `SafeArrayDestroy`ed, which destroys + the array descriptor and all data: `VariantClear` on `VARIANT`s, + `SysFreeString` on `BSTR`s, `IRecordInfo::RecordClear` on records, + `Release` on `IUnknown`s. + """ + ptr = POINTER(self._itemtype_)() # container for the values + # Make an independent `SAFEARRAY` so its data does not depend on + # the lifetime of the source `SAFEARRAY`, which is `self`. + copied = _safearray.SafeArrayCopy(self) # type: ignore + # Setting this flag makes `__del__` call `SafeArrayDestroy` + # when the Python wrapper is finalized. + copied._needsfree = True + # Lock the copied `SAFEARRAY` and get its native record storage. + # A direct `ptr` avoids making a separately owned record copy. + _safearray.SafeArrayAccessData(copied, byref(ptr)) + try: + + def keep_safearray(v): + v1 = v.__class__() + # Copy bytes, not pointed-to data; `copied` owns that data. + memmove(byref(v1), byref(v), sizeof(v)) + # This strong reference keeps `copied` alive while `v1` + # is alive, so `copied.__del__` cannot run and destroy + # the SAFEARRAY. A weak reference would not do this. + v1.__keepref = copied + return v1 + + # Return only the requested number of records. + # Each result keeps the `copied` wrapper alive. + return [keep_safearray(x) for x in ptr[:num_elements]] + finally: + # Unlock the native SAFEARRAY storage; this does not destroy it. + _safearray.SafeArrayUnaccessData(copied) + def _get_row(self, dim, indices, lowerbounds, upperbounds): # loop over the index of dimension 'dim' # we have to restore the index of the dimension we're looping over From a83730138bceaf6029c7ead9f3891898f7ff0927 Mon Sep 17 00:00:00 2001 From: junkmd Date: Fri, 6 Nov 2026 07:42:16 +0900 Subject: [PATCH 5/8] docs: Improve `_get_records` docstring for `VT_RECORD` lifetime management. --- comtypes/safearray.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/comtypes/safearray.py b/comtypes/safearray.py index 3aa2e019c..f72c384f7 100644 --- a/comtypes/safearray.py +++ b/comtypes/safearray.py @@ -385,9 +385,8 @@ def _get_records(self, num_elements: int) -> Sequence[Structure]: Once the array and all element records are GC'd in Python, the copied array is automatically `SafeArrayDestroy`ed, which destroys - the array descriptor and all data: `VariantClear` on `VARIANT`s, - `SysFreeString` on `BSTR`s, `IRecordInfo::RecordClear` on records, - `Release` on `IUnknown`s. + the array descriptor and all data: `IRecordInfo::RecordClear` on + records. """ ptr = POINTER(self._itemtype_)() # container for the values # Make an independent `SAFEARRAY` so its data does not depend on From ac284718f20d1951c1494e77b3cb438637970b3f Mon Sep 17 00:00:00 2001 From: junkmd Date: Fri, 6 Nov 2026 07:42:16 +0900 Subject: [PATCH 6/8] fix: `VT_ARRAY|VT_RECORD` handling in `tagVARIANT._get_value`. Problem: - `self.vt & VT_RECORD` is wrong because `VT_RECORD` (36) is a value, not a flag. - Bitwise AND with `VT_RECORD` can be true for unrelated vt like `VT_INT` (22). Solution: - Change the check to `(self.vt & ~VT_ARRAY) == VT_RECORD`. - This correctly checks if the base type (without `VT_ARRAY` flag) is `VT_RECORD`. - Remove redundant code and simplify the logic. --- comtypes/automation.py | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/comtypes/automation.py b/comtypes/automation.py index da851a1f2..1372051f7 100644 --- a/comtypes/automation.py +++ b/comtypes/automation.py @@ -697,27 +697,22 @@ def _get_value(self, dynamic=False): except KeyError: pass - if self.vt & VT_RECORD: + if (self.vt & ~VT_ARRAY) == VT_RECORD: from comtypes.client import GetModule - from comtypes.typeinfo import IRecordInfo # Get generic safearray and its IRecordInfo - ri0 = cast(self._.pparray, POINTER(_safearray.SAFEARRAY)) - ri1 = _safearray.SafeArrayGetRecordInfo(ri0) - # QUESTION: Do we need to add a ref ? - ri2 = ri1.QueryInterface(IRecordInfo) + pa = cast(self._.pparray, POINTER(_safearray.SAFEARRAY)) + ri = _safearray.SafeArrayGetRecordInfo(pa) # Now containing typelib and through it the right class type - mod = GetModule(ri2.GetTypeInfo().GetContainingTypeLib()[0]) - ricls = getattr(mod, ri2.GetName()) + mod = GetModule(ri.GetTypeInfo().GetContainingTypeLib()[0]) + ricls = getattr(mod, ri.GetName()) # Cast the array to the right safearray(type), unpack -> return return cast(self._.pparray, _midlSAFEARRAY(ricls)).unpack() # Else we cannot unpack the array - raise NotImplementedError("typecode %d = 0x%x)" % (vt, vt)) - else: - raise NotImplementedError(f"typecode {vt} = 0x{vt:x})") + raise NotImplementedError(f"typecode {vt} = 0x{vt:x})") def __getitem__(self, index): if index != 0: From bf1b5ddd4af77a79749c7db767ad71913c00bcf9 Mon Sep 17 00:00:00 2001 From: junkmd Date: Fri, 6 Nov 2026 07:42:16 +0900 Subject: [PATCH 7/8] fix: Replace unreachable `memmove` branch with `NotImplementedError` for `Structure` without `_recordinfo_`. After adding the `is_struct_type_with_recordinfo` early-return in `_get_elements_raw`, the else branch reached by a `Structure` subtype without `_recordinfo_` could no longer be handled correctly by the remaining `memmove`-based `keep_safearray`. A `Structure` `SAFEARRAY` that lacks `IRecordInfo` is not constructable through normal COM codegen: the COM spec requires `IRecordInfo` for any `VT_RECORD` array. Silently returning data from a shallow byte-copy risks double-free crashes for fields with `BSTR` or pointer semantics. Raising `NotImplementedError` makes the unsupported state explicit instead of returning potentially corrupt data, and signals to future maintainers that a safe extraction path needs to be designed if this case ever becomes real. --- comtypes/safearray.py | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/comtypes/safearray.py b/comtypes/safearray.py index f72c384f7..7079038e4 100644 --- a/comtypes/safearray.py +++ b/comtypes/safearray.py @@ -357,15 +357,17 @@ def _get_elements_raw(self, num_elements): return arr.copy() return ptr[:num_elements] - def keep_safearray(v): - # Simply keeping a reference to self does not keep the - # internal addresses of BSTR safe and strings will be - # overwritten. A copy of the bytes solves the problem - v1 = v.__class__() - memmove(byref(v1), byref(v), sizeof(v)) - return v1 - - return [keep_safearray(x) for x in ptr[:num_elements]] + # A Structure subtype without `_recordinfo_` cannot represent + # a valid COM SAFEARRAY of VT_RECORD: the COM specification + # requires IRecordInfo to describe the record layout. + # Reaching here means the type was constructed outside the + # normal COM codegen path. No safe extraction strategy + # exists, so raise rather than silently returning corrupt data. + raise NotImplementedError( + f"{self._itemtype_!r} is a Structure without" + " _recordinfo_; safe extraction from SAFEARRAY is not" + " supported." + ) finally: _safearray.SafeArrayUnaccessData(self) From cb361e1066b684378be761a189fa2e65514da312 Mon Sep 17 00:00:00 2001 From: junkmd Date: Fri, 6 Nov 2026 07:42:16 +0900 Subject: [PATCH 8/8] refactor: Simplify `VT_RECORD` check in `safearray`. In `_get_elements_raw`, use `self._vartype_ == VT_RECORD` instead of `is_struct_type_with_recordinfo(self._itemtype_)`. The `_vartype_` is set to `VT_RECORD` when the item type is a `Structure` with valid `_recordinfo_`. This check is done in `_make_safearray_type`. Benefits: - No function call needed - No import from typeinfo needed - Simple integer comparison is faster - Code is shorter and clearer Also remove unused `is_struct_type_with_recordinfo` and `_is_recordinfo_guids` from `typeinfo.py` because they are not used anywhere now. --- comtypes/safearray.py | 3 +-- comtypes/typeinfo.py | 34 ---------------------------------- 2 files changed, 1 insertion(+), 36 deletions(-) diff --git a/comtypes/safearray.py b/comtypes/safearray.py index 7079038e4..9235f99b7 100644 --- a/comtypes/safearray.py +++ b/comtypes/safearray.py @@ -300,9 +300,8 @@ def _get_elements_raw(self, num_elements): """Returns a flat list or ndarray containing ALL elements in the safearray.""" from comtypes.automation import VARIANT, tagCY, tagDEC - from comtypes.typeinfo import is_struct_type_with_recordinfo - if is_struct_type_with_recordinfo(self._itemtype_): + if self._vartype_ == VT_RECORD: return self._get_records(num_elements) # XXX Not sure this is true: # For VT_UNKNOWN and VT_DISPATCH, we should retrieve the diff --git a/comtypes/typeinfo.py b/comtypes/typeinfo.py index 7f7e1067a..200d7ca68 100644 --- a/comtypes/typeinfo.py +++ b/comtypes/typeinfo.py @@ -663,40 +663,6 @@ def GetRecordInfoFromTypeInfo(tinfo: ITypeInfo) -> IRecordInfo: return ri # type: ignore -_RIGuids = tuple[str, int, int, int, str] - - -def _is_recordinfo_guids(obj: Any) -> "hints.TypeIs[_RIGuids]": - if not isinstance(obj, tuple): - return False - if len(obj) != 5: - return False - tlibid, majorver, minver, lcid, tinfoid = obj - if ( - isinstance(tlibid, str) - and isinstance(majorver, int) - and isinstance(minver, int) - and isinstance(lcid, int) - and isinstance(tinfoid, str) - ): - return True - return False - - -# TODO: This should be typed as an intersection of structure and protocol. -def is_struct_type_with_recordinfo(typ: type[Any]) -> bool: - """Return `True` if `typ` is a `Structure` type with valid `_recordinfo_` - class variable. - """ - if not issubclass(typ, ctypes.Structure): - return False - if not hasattr(typ, "_recordinfo_"): - return False - if not _is_recordinfo_guids(typ._recordinfo_): - return False - return True - - def GetRecordInfoFromGuids( rGuidTypeLib: str, verMajor: int, verMinor: int, lcid: int, rGuidTypeInfo: str ) -> IRecordInfo: