Fix SAFEARRAY ownership for VT_ARRAY | VT_RECORD values.

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]

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:

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

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
 
@@ -300,6 +301,8 @@ def _get_elements_raw(self, num_elements):
             the safearray."""
             from comtypes.automation import VARIANT, tagCY, tagDEC
 
+            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
             # interface iid by SafeArrayGetIID().
@@ -353,18 +356,68 @@ 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)
 
+        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: `IRecordInfo::RecordClear` on
+            records.
+            """
+            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

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__":