[PyQt] Fwd: QModelIndex.internalPointer Bug

[Martin Teichmann]

Hi Andres, Hi Phil, Hi list,

> The behavior of the bug is that python crashes when you create an index with
> a pointer to an instantiated object's attribute, and then try to access that
> pointer using the internalPointer method.

The problem is that this is not what you're doing in your example.
createIndex with an integer as a parameter does not create a reference
to that integer, but actually stores the integer into the model index.
When you get it out with internalPointer it is re-interpreted as a
pointer, in this case to memory address 2, and this gives a segfault.

All of this is a big nightmare. You have to have disciplin in what you
put into a QModelIndex, as upon retrieving it there is no way to tell
what type it originally was. And the really bad thing about it is that
you are punished with a SEGFAULT, not just a Python exception.

It gets even worse: there is no way to tell whether Qt destroyed a
QModelIndex. This kills Python reference counting, as we don't know
when to decrease it. That's why PyQt does not increase the Python
reference counter for an objected entered into createIndex. This also
means that upon retrieving it with internalPointer may crash with a
SEGFAULT if the object referenced to has been collected in the
meantime.

For the longest time, I thought there is no solution to this problem.
But finally I solved it for my code: I store all objects that go into
a WeakValueDictionary, and only hand over the id to Qt. About like
that:

    objects = WeakValueDictionary()

    def createIndex(model, row, col, obj):
        objects[id(obj)] = obj
        return model.createIndex(row, col, id(obj))

    def retrieveObject(index):
        # this will return None if the object has already been collected
        return objects.get(index.internalId())

This works well without SEGFAULT. I think that it would be a good idea
for PyQt to use a model like that internally, as it would stop
programs from segfaulting. It would be nice if this was even the
standard behavior of internalPointer. This would indeed change the API
in a potentially incompatible way. But this incompatibility would only
be that a program doesn't segfault but return a None. The only problem
is that not everything can be put into a WeakValueDictionary. This
could be mitigated by remembering what could not be weak referenced,
in this case the code would still SEGFAULT, for backwards
compatibility...

Written in Python:

    objects = WeakValueDictionary()
    non_weak_ids = set()

    def createIndex(model, row, col, obj):
        if isinstance(obj, int):  # for those who prefer to store ids
            return model.createIndex(row, col, obj):
        try:
            objects[id(obj)] = obj
            return model.createIndex(row, col, id(obj))
        except TypeError:  # weak ref could not be created
            non_weak_ids.add(id(obj))
            return model.createIndex(row, col, obj)

    def internalPointer(index):
        ret = objects.get(index.internalId())
        if ret is None and index.internalId() in non_weak_ids:
            return index.internalPointer()  # this may SEGFAULT!
        return ret

This would be a backwards-compatible implementation.

Greetings

Martin