Python extension creates invalid pointers when manipulating large lists

There are more problems with your extension function outside of reference counting errors, more of them below:

While PyList_SetItem

correct reference counting is the preferred way, the (ugly) option is to use a macro PyList_SET_ITEM

that goes away with the INCREF:

void PyList_SET_ITEM(PyObject *list, Py_ssize_t i, PyObject *o)

Macro formation PyList_SetItem()

without error checking. This is usually only used to populate new listings that do not have previous content.

Note

This macro "steals" a reference to an element and, unlike PyList_SetItem()

, does not discard a reference to any element that is being replaced; any link listed in position i

will be leaked.

This way, it PyList_SET_ITEM

does not increase or decrease any reference counters that are appropriate for us, since initially and finally the items are in the same list.

inline void _List_SwapItems(PyObject* list, Py_ssize_t i1, Py_ssize_t i2){
    PyObject* tmp = PyList_GET_ITEM(list, i2);
    PyList_SET_ITEM(list, i2, PyList_GET_ITEM(list, i1));
    PyList_SET_ITEM(list, i1, tmp);
}

      

        
        
        
      

    

Note that this does not do any error checking at all, so you need to make sure your index is within the limits (which the loop handles for

).

There is another bad problem in your code that is not discussed yet - the complete lack of error checking. For example, when passing in an object that is not a list, you must raise TypeError

. Now the code will be PyList_Size

, by returning -1 and setting an inner exception, this could lead to erroneous behavior for all future C extensions:

Likewise, it PyArg_ParseTuple

may fail if the wrong number of arguments is passed to , so you should check its return value; in this case it list

can be uninitialized and your code will have full undefined behavior.

The C-API documentation states the following :

If a function should fail because some function called by it failed, it usually does not set an error indicator; the function she was calling had already installed it. It is responsible for handling errors and clearing an exception or returning after clearing any resources (such as object references or memory allocations); it shouldn't proceed normally unless it's ready to handle the error. If returning due to an error, it is important to indicate to the caller that an error has been fixed. If an error is not handled or carefully additional calls in the Python / C API may not be as intended and may fail in mysterious ways.

So here's the correct way to write your extension function:

static PyObject* shuffle(PyObject* self, PyObject* args){
    PyObject* list;
    if (! PyArg_ParseTuple(args, "O", &list)) {
        // PyArg_ParseTuple set the proper exception
        return NULL;
    }

    if (! PyList_Check(list)) {
        PyErr_SetString(PyExc_TypeError,
            "bad argument to shuffle; list expected");
        return NULL;
    }

    unsigned seed = std::chrono::system_clock::now().time_since_epoch().count();
    std::minstd_rand0 rand(seed);

    Py_ssize_t size = PyList_Size(list);
    for(int i=0; i<size;++i){
        int randIndex = rand()%size;
        _List_SwapItems(list, randIndex, i);
    }
    Py_RETURN_NONE;
}