SIP - Error in mapped type

[Phil Thompson]

On 23/04/2021 08:04, Marian Thomsen wrote:
> Hello,
> 
> the following mapped type caused the error "free(): double free
> detected in tcache 2" which I cannot find. Can someone tell me what
> I'm missing?
> 
> 
> PathPoint is a wrapped struct containing some other wrapped structs.
> The content can also only be an unsigned int and the error is still
> there.
> 
> 
> When I set
> 
> gc.set_debug(gc.DEBUG_LEAK)
> 
> (meaning postponed garbage collection) in the python script that uses
> this wrapped vector, the error does not appear.
> 
> My guess is that it has something to do with "PathPoint *t" or the
> ownership of "item" in ConvertToTypeCode.
> 
> 
> 
> %MappedType std::vector<PathPoint *>
> {
> %TypeHeaderCode
> #include <vector>
> %End
> 
> %ConvertFromTypeCode
>     PyObject *l;
> 
>     const sipTypeDef* kpTypeDef = sipFindType("PathPoint");
> 
>     if (!kpTypeDef) {
>         return NULL;
>     }
> 
>     // Create the Python list of the correct length.
>     if ((l = PyList_New(sipCpp->size())) == NULL) {
>         return NULL;
>     }
> 
>     int i = 0;
>     // Go through each element in the C++ instance and convert it to
> the corresponding Python object.
>     for(std::vector<PathPoint *>::iterator iter = sipCpp->begin();
> iter != sipCpp->end(); iter++) {
>         PathPoint *t = *iter;
>         PyObject *tobj;
> 
>         // Get the Python wrapper for the Type instance, creating a new
>         // one if necessary, and handle any ownership transfer.
>         if ((tobj = sipConvertFromType(t, kpTypeDef, sipTransferObj)) 
> == NULL) {
>             // There was an error so garbage collect the Python list.
>             Py_XDECREF(l);
>             return NULL;
>         }
> 
>         // Add the wrapper to the list.
>         PyList_SET_ITEM(l, i++, tobj);
>     }
> 
>     // Return the Python list.
>     return l;
> %End
> 
> %ConvertToTypeCode
>     const sipTypeDef* kpTypeDef = sipFindType("PathPoint");
> 
>     if (!kpTypeDef) {
>         return 0;
>     }
> 
>     // See if we are just being asked to check the type of the Python 
> object.
>     if (sipIsErr == NULL) {
>         // Check it is a list.
>         if (!PyList_Check(sipPy)) {
>             return 0;
>         }
> 
>         // Now check each element of the list is of the type we expect.
>         // The template is for a pointer type so we don't disallow 
> None.
>         for (Py_ssize_t i = 0; i < PyList_GET_SIZE(sipPy); ++i) {
>             PyObject *item = PyList_GET_ITEM(sipPy, i);
>             if (!sipCanConvertToType(item, kpTypeDef, SIP_NOT_NONE)) {
>                 *sipIsErr = 1;
>                 break;
>             }
>         }
>         return 1;
>     }
> 
>     // Create the instance on the heap.
>     std::vector<PathPoint *> *v = new std::vector<PathPoint *>();
> 
>     for (Py_ssize_t i = 0; i < PyList_GET_SIZE(sipPy); ++i) {
>         int state;
>         // Use the SIP API to convert the Python object to the
>         // corresponding C++ instance.  Note that we apply any 
> ownership
>         // transfer to the list itself, not the individual elements.
>         PyObject *item = PyList_GET_ITEM(sipPy, i);
>         PathPoint *t = static_cast<PathPoint*>(sipConvertToType(item,
> kpTypeDef, sipTransferObj,
> 
> SIP_NOT_NONE, &state, sipIsErr));
> 
>         if (*sipIsErr) {
> 
>             sipReleaseType(t, kpTypeDef, state);
> 
>             Py_DECREF(item);
> 
>             // Tidy up.
>             delete v;
> 
>             // There is nothing on the heap.
>             return 0;
>         }
> 
>         // ownership to cpp (no Py_DECREF needed)
>         sipTransferTo(item, item);
> 
>         // Add the pointer to the C++ instance.
>         v->push_back(t);
> 
>         sipReleaseType(t, kpTypeDef, state);
>     }
> 
>     // Return the instance on the heap.
>     *sipCppPtr = v;
> 
>     // Apply the normal transfer.
>     return sipGetState(sipTransferObj);
> %End
> };

In the comments in the 'from' code you say you are applying ownership 
transfer to each element, and the code you have written does that.

In the comments in the 'to' code you say you are applying ownership 
transfer to the list and not each element - so there is an inconsistency 
there. However the code you have written does the transfer for the list, 
*and* for each element, *and* again for each element (the explicit call 
to sipTransferTo().

Phil