connect()'s refcounting behavior seems buggy, and should be documented

[Phil Thompson]

On 17/03/2022 00:54, Ben Rudiak-Gould wrote:
> As far as I can tell from experimentation (see code below), the 
> refcounting
> behavior of connect() is:
> 
> * If the receiver is a bound method, a weak reference is held to the 
> object
> and a bare pointer is held to the function. If the object is collected, 
> the
> connection is automatically destroyed; if the function is collected 
> (del
> Class.method), emit() segfaults.
> 
> * If the receiver is any other callable, a strong reference to it is 
> held
> and leaked. It is never released even when the connection is destroyed.
> 
> That behavior seems to be consistent across PyQt 4, 5, and 6.
> 
> The behavior of PySide is:
> 
> * If the receiver is a bound method, PySide and PySide2 behave like 
> PyQt
> (including the crash), but PySide6 holds a strong reference to the 
> function
> and releases it when the connection is destroyed.
> 
> * If the receiver is any other callable, all PySide versions hold a 
> strong
> reference to it and release it when the connection is destroyed.
> 
> I think that PyQt should do what PySide6 does.
> 
> Aside from crash avoidance always being good, holding a reference to 
> the
> function of a bound method would allow for code like
> 
>     obj1.signal.connect((lambda obj2: ...).__get__(obj2))
> 
> to make a connection to what appears to Qt to be a method of obj2. 
> There's
> a message on this list asking how to do that, which got no replies:
> https://www.riverbankcomputing.com/pipermail/pyqt/2020-November/043390.html
> 
> 
> There seems to be no description of connect()'s refcounting behavior in 
> the
> documentation of any version of PyQt or PySide, except for an 
> inaccurate
> paragraph in PyQt4's page on old-style SIGNAL and SLOT. The fact that 
> bound
> methods are treated differently from all other callables is non-obvious 
> and
> important, and I think it should be mentioned prominently.

I've changed the PyQt6 behaviour to keep a reference to bound methods. 
I'm not going to change PyQt5 as stability (avoiding unexpected 
consequences) is more important than correctness so late in its 
lifecycle.

> Test code:
> 
>     #from PyQt4.QtCore import QObject, pyqtSignal as Signal
>     #from PyQt5.QtCore import QObject, pyqtSignal as Signal
>     from PyQt6.QtCore import QObject, pyqtSignal as Signal
>     #from PySide.QtCore import QObject, Signal
>     #from PySide2.QtCore import QObject, Signal
>     #from PySide6.QtCore import QObject, Signal
> 
>     class Source(QObject):
>         signal = Signal()
> 
>         def __del__(self):
>             print('Source deleted')
> 
>     class Sink:
>         def __call__(self):
>             print('Sink called')
> 
>         def __del__(self):
>             print('Sink deleted')
> 
>     source = Source()
>     sink = Sink()
> 
>     if False:  # segfaults except on PySide6
>         Sink.foo = lambda self: print('Ephemeral method called')
>         source.signal.connect(sink.foo)
>         del Sink.foo
>         source.signal.emit()
> 
>     source.signal.connect(sink)
>     del source
>     del sink
> 
> In all PyQt versions this prints only "Source deleted". In all PySide
> versions it also prints "Sink deleted".
> 
> I tried making Sink inherit from QObject, making the Sink method a 
> slot,
> explicitly destroying the connection with disconnect(), and forcing 
> garbage
> collection, but it made no difference.

It's a question of timing - you don't have an event loop. Experience has 
shown that you have to be very careful exactly when you disconnect and 
delete the proxies around callables.

Phil