[PyQt] Python owned QObject released from a thread with pending Meta call events.

Ales Erjavec ales.erjavec324 at gmail.com
Wed Apr 15 10:49:48 BST 2020


Hi,

I have been struggling with segfaults when using QObject subclass as a
queued communicator between threads.

I think there is an unsafe access in the implementation of
sipQObject::qt_metacall, qpycore_qobject_qt_metacall and the
`dealloc_QObject`, when the last reference to the sip python wrapper
object is released and deallocated from a thread that is not its own
(i.e. QThread().currentThread() is not qobj.thread())

In particular; dealloc_QObject:

```
static void dealloc_QObject(sipSimpleWrapper *sipSelf)
{
    if (sipIsDerivedClass(sipSelf))
        reinterpret_cast<sipQObject
*>(sipGetAddress(sipSelf))->sipPySelf = SIP_NULLPTR;

    if (sipIsOwnedByPython(sipSelf))
    {
        release_QObject(sipGetAddress(sipSelf), sipIsDerivedClass(sipSelf));
    }
}
```

Here the pointer from sipQObject to sip python wrapper (sipPySelf) is
cleared (this happens with the GIL held), but when
the current thread is not the qobjects's thread the sipQObject's
deletion is scheduled by deleteLater() in release_QObject:

```
static void release_QObject(void *sipCppV, int)
{
     ::QObject *sipCpp = reinterpret_cast< ::QObject *>(sipCppV);

    if (QThread::currentThread() == sipCpp->thread())
        delete sipCpp;
    else
        sipCpp->deleteLater();
}
```

Say however that the object has pending meta call events
(slots/signals scheduled via Qt's meta object system e.g. signal/slot
connections using Qt.QueuedConnection). Such events eventually get to:

```
int sipQObject::qt_metacall(QMetaObject::Call _c,int _id,void **_a)
{
    _id =  ::QObject::qt_metacall(_c,_id,_a);

    if (_id >= 0)
        _id = sip_QtCore_qt_metacall(sipPySelf,sipType_QObject,_c,_id,_a);

    return _id;
}
```

This is called without the GIL held, and reads the value of sipPySelf
to pass to `sip_QtCore_qt_metacall` (alias for
`qpycore_qobject_qt_metacall`)

```
int qpycore_qobject_qt_metacall(sipSimpleWrapper *pySelf, sipTypeDef *base,
        QMetaObject::Call _c, int _id, void **_a)
{
    // Check if the Python object has gone.
    if (!pySelf)
        return -1;

    SIP_BLOCK_THREADS
    _id = qt_metacall_worker(pySelf, Py_TYPE(pySelf), base, _c, _id, _a);
    SIP_UNBLOCK_THREADS

    return _id;
}

```
Between the point in sipQObject::qt_metacall where the sipPySelf is
read and SIP_BLOCK_THREADS in `qpycore_qobject_qt_metacall` the python
wrapper can be deallocated, leaving pySelf on the stack a dangling
pointer (in fact the memory it points to could have already been
reused).

I think the entire
`sip_QtCore_qt_metacall(sipPySelf,sipType_QObject,_c,_id,_a)` call in
sipQObject::qt_metacall needs to be inside a
SIP_BLOCK/UNBLOCK_THREADS.

I am using this script to test and trigger a segfault, but I am not
sure how 'portable' it is since this is quite timing sensitive:

```
import time
from types import SimpleNamespace
from threading import Event
from concurrent.futures.thread import ThreadPoolExecutor

from PyQt5.QtCore import (
    Qt, QObject, QCoreApplication, QTimer, QEventLoop, pyqtSignal
)


class Emitter(QObject):
    signal = pyqtSignal()
    _p_signal = pyqtSignal()
    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        # queued signal -> signal connection
        self._p_signal.connect(self.signal, Qt.QueuedConnection)

    def schedule_emit(self):
        """Schedule `signal` emit"""
        self._p_signal.emit()

app = QCoreApplication([])
executor = ThreadPoolExecutor(max_workers=4)

def test():
    ref = SimpleNamespace()  # hold single reference to Emitter obj
    ref.obj = Emitter()
    # enqueue 200 meta call events to the obj
    for i in range(200):
        ref.obj.schedule_emit()

    # event signaling the event loop is about to be entered
    event = Event()

    def clear_obj(ref=ref):
        # wait for main thread to signal it is about to enter the event loop
        event.wait()
        # time.sleep(0) # yield thread
        del ref.obj  # clear the last/single ref to obj

    executor.submit(clear_obj)

    loop = QEventLoop()
    QTimer.singleShot(0, loop.quit)
    # bytecode optimizations, reduce the time between event.set and exec to
    # minimum
    set = event.set
    exec = loop.exec

    set()    # signal/unblock the worker;
    exec()   # enter event loop to process the queued meta calls


iter_n = 0

while True:
    if iter_n % 10 == 0:
        print("iter:", iter_n, flush=True)
    test()
    iter_n += 1

```

Best wishes
    Aleš Erjavec


More information about the PyQt mailing list