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

[Ales Erjavec]

One possibility might be to 'resurrect' the python wrapper from its
finalizer, postponing its deallocation until the 'delete later' event
is processed.

In python equivalent:
```
...
delete_later_set = set()
...

    def __del__(self):
        if sip.ispyowned(self):
            try:
                own_thread = self.thread() is QThread.currentThread()
            except RuntimeError:
                return
            if not own_thread:
                # object resurrection
                delete_later_set.add(self)
                self.deleteLater()

    def __dtor__(self):
        # called from c++ destructor
        try:
            delete_later_set.remove(self)
        except KeyError:
            pass

```

On Wed, Apr 15, 2020 at 11:49 AM Ales Erjavec <ales.erjavec324 at gmail.com> wrote:
>
> 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