Regression in passing buffers through void * with Python 3.13

[guitargeek]

Reproducer:

import array
import cppyy

cppyy.cppdef("""

double* pass_arr(double* a) {
   return a;
}
double* pass_void_arr(void* a) {
   return (double*)a;
}

""");

b = array.array('d', range(6))

# Works!
ca = cppyy.gbl.pass_arr(b)
for i in range(len(b)):
    print(ca[i])

# Crashes!
ca = cppyy.gbl.pass_void_arr(b)
for i in range(len(b)):
    print(ca[i])

I investigated the problem a bit, and it turns out the IsCTypesArrayOrPointer() is giving a false positive, resulting in the wrong code path being taken:
https://github.com/wlav/CPyCppyy/blob/master/src/Converters.cxx#L1510

@wlav, do you have any idea on how to improve this function? I don't understand its implementation, unfortunately. How does relate comparing the type of the extended dictionary (Py_TYPE(pytype->tp_dict)) into figuring out whether it's a ctype or not? Is the PyTYPE of a dictionary not always a dictionary anyway, therefore resulting in the false positive?

[wlav]

The reason why it worked was that ctypes' types have had a custom dictionary type (StgDictObject) since their introduction in py2.5 and ctypes code has been pretty much left untouched until, apparently, now in py3.13. (Looks like the custom info is now stored directly in the type object.) Checking the custom dictionary provided a single test for all ctypes types that was convenient and fast.

From the py3.13 code, the proper way of checking would now be a PyObject_IsInstance for the (newly introduced) PyCType_Type common type base class.

[guitargeek]

Ah ok I suspected something like that! Great that you found the PyCType_Type, I missed that when looking around the Python docs yesterday.

[wlav]

It's not in the docs (I'm sure; haven't actually looked) b/c ctypes does not have a public C API.

[wlav]

It's actually not used as a type base class, but as a meta type and their doesn't seem to be any hierarchy of meta types. Ie., a simple test w/o needing to copy/expose any functions from ctypes.h would be to compare the type of the type of any ctypes type (no isinstance() needed).

[wlav]

Looks like there was more to it than just convenience: the dict-checking trick also covered pointer types which are created at runtime and which are not part (as such) of ctypes. The metaclass-checking as proposed does not cover them.

[wlav]

Should work now. I found no better way than to copy the state definition. I'd rather pick the metatype from one of the existing types (as that would not involve code copying), but the modules (and types) associated with pointers are those where and from what they are created at run-time, making that unreliable. (Note that even for getting the state, the relevant module is _ctypes, not ctypes.)

[guitargeek]

Thank you so much for you updates!

With my previous hotfixes, we could already add Fedora 41 with Python 3.13 in the CI. I'm checking now if everything works with your solution:

• [CPyCppyy] Fix regression in passing buffers through void * root-project/root#16861

[wlav]

I'm down to 2 leak checks failing on py3.13 that don't under other pythons; then 4 more tests that are failing b/c of Clang 16 (3 related to not keeping int8_t as a typedef and 1 to lambdas; both work only on my end due to changes in ROOT/meta, so probably never worked for ROOT main anyway).

[guitargeek]

Good to hear! On our side, I don't see any regressions anymore due to Python 3.13 for whatever test coverage we have :) So we're shifting the effort again towards general PyROOT improvements. Especially for the memory regulator, that regular comes to bite us (see root-project/root#16837). We also discussed with Vassil at the last Tuesday meeting that we have to do a general revamp of how ROOT is initialized, since it's too implicit.