Fix memory leak in Downloads page

[ichorid]

Fixes #5934

The moral of the story: if you connect a QT signal to a QT object, don't expect it to be GCed in PyQT until you disconnect the signal manually (or better yet, just reuse the object).

[ichorid]

retest this please

[sonarcloud]

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

No Coverage information
0.0% Duplication

[drew2a]

@ichorid congrats! 🎊

But I have a question. Are you sure about " don't expect it to be GCed in PyQT until you disconnect the signal manually"?

Because:

To connect slots to it, pass callbacks into connect. The connections are maintained through weakref, thus you don't need to search for them and disconnect whenever you're up to destroy some object.
https://pypi.org/project/qsignal/

BTW yes, we always should help GC: spesmilo/electrum#4905

[ichorid]

@ichorid congrats! confetti_ball

But I have a question. Are you sure about " don't expect it to be GCed in PyQT until you disconnect the signal manually"?

I learned the hard way never to trust PyQT promises... 😿

[drew2a]

@ichorid @qstokkink what do you think, do we have to use something like:

class QAutoDisconnectableObject(QObject):
    def __init__(self):
        super().__init__()

        self.connected_signals = set()

        connect(self.destroyed, self.on_destroy)

    def connect(self, signal, callback):
        connect(signal, callback)
        self.connected_signals.add(signal)

    def on_destroy(self, *args):
        for signal in self.connected_signals:
            signal.disconnect()


class Table(QAutoDisconnectableObject):
    def __init__(self):
        super().__init__()
        self.connect(self.button.clicked, self.on_button_clicked)
        self.connect(self.button1.clicked, self.on_button1_clicked)

[qstokkink]

@drew2a It seems useful to have a construction like that: I don't see any downsides to your suggestion.

[ichorid]

https://www.sep.com/blog/prevent-signal-slot-memory-leaks-in-python/