3

Please consider the minimal example below, which implements a custom QNetworkAccessManager that maintains a list of unfinished QNetworkReply instances.

When a reply is finished, it is removed from the unfinished_replies list.

As discussed in Is deleteLater() necessary in PyQt/PySide?, QNetworkReply.deleteLater() is used, inside the finished slot, to schedule the Qt object for deletion.

However, I am not sure what would be the best way to remove the Python reference to the reply object. I can think of two (mutually exclusive) options for removing the Python reference, as shown in the example below:

  1. remove directly after calling deleteLater()

  2. remove when the QNetworkReply.destroyed signal is emitted (docs)

Both options seem to work just fine. I would prefer option 1, but I'm not sure if this could lead to surprises in rare cases. Which would be best? Or is there another alternative?

import sys
from PyQt5 import QtNetwork, QtWidgets, QtCore


class CustomNetworkAccessManager(QtNetwork.QNetworkAccessManager):
    def __init__(self):
        super(CustomNetworkAccessManager, self).__init__()
        self.unfinished_replies = []
        self.finished.connect(self.slot)

    def get(self, *args, **kwargs):
        reply = super(CustomNetworkAccessManager, self).get(*args, **kwargs)
        reply.index = i  # just for printing
        self.unfinished_replies.append(reply)

    def remove_from_list(self, reply):
        self.unfinished_replies.remove(reply)
        print('{} unfinished replies left'.format(len(self.unfinished_replies)))
        if not self.unfinished_replies:
            QtCore.QCoreApplication.quit()

    def slot(self, reply):
        print('reply {} finished'.format(reply.index))
        # handle the Qt side:
        reply.deleteLater()  
        # handle the Python side:
        # either
        # OPTION 1 - remove now
        self.remove_from_list(reply)
        # or 
        # OPTION 2 - remove when destroyed
        # reply.destroyed.connect(lambda: self.remove_from_list(reply))


if __name__ == '__main__':
    # Initialize
    app = QtWidgets.QApplication(sys.argv)
    manager = CustomNetworkAccessManager()

    # Schedule requests
    url = 'http://httpbin.org/get'
    for i in range(6):
        manager.get(QtNetwork.QNetworkRequest(QtCore.QUrl(url)))

    # Start event loop
    app.exec_()

p.s. sorry for the Python 2 code

djvg
  • 11,722
  • 5
  • 72
  • 103

1 Answers1

3

Both are equivalent, they only differ the moment they are removed. But to understand more in detail you have to understand how the PyQt5/PySide2 binding works. Those libraries create a wrapper around the C++ object, something like:

class FooWrapper:
    def __new__(self, cls, *args, **kwargs):
         # ...
         instance = ...
         instance._cpp_object = create_cpp_object()
         # ...
         return instance

    def foo_method(self, *args, **kwargs):
        return execute_foo_method(self._cpp_object, *args, **kwargs)

    def __del__(self):
        destroyed_cpp_object(self._cpp_object)

So when calling deleteLater only the cpp_object is deleted and not the wrapper, you can verify that if you use:

reply.destroyed.connect(self.remove_from_list)
Traceback (most recent call last):
  File "main.py", line 32, in <lambda>
    reply.destroyed.connect(self.remove_from_list)
  File "main.py", line 17, in remove_from_list
    self.unfinished_replies.remove(reply)
ValueError: list.remove(x): x not in list

since the parameter passed by destroyed is an invalid wrapper getting the above error. For this case, a solution is to check if the cpp_object has been removed with sip.isdeleted():

from PyQt5 import QtNetwork, QtWidgets, QtCore
import sip

# ...
class CustomNetworkAccessManager(QtNetwork.QNetworkAccessManager):
    # ...

    def remove_from_list(self):
        self.unfinished_replies = [
            reply for reply in self.unfinished_replies if not sip.isdeleted(reply)
        ]
        print("{} unfinished replies left".format(len(self.unfinished_replies)))
        if not self.unfinished_replies:
            QtCore.QCoreApplication.quit()

    def slot(self, reply):
        print("reply {} finished".format(reply.index))
        # handle the Qt side:
        reply.deleteLater()
        # handle the Python side:
        reply.destroyed.connect(self.remove_from_list)

Returning to the study of your methods, these can be graphed as follows:

(FIRST METHOD)
------------┬------------------┬---------------------┬-----------------------
            |                  |                     |
 call_deleteLater remove_reply_from_list          destroyed
(SECOND METHOD)
------------┬-----------------------------------------┬-----------------┬------
            |                                         |                 |
 call_deleteLater                               destroyed remove_reply_from_list

And since you are using the original wrappers you should not have any problem.

Conclusion: Both are equivalent and safe.

eyllanesc
  • 235,170
  • 19
  • 170
  • 241
  • Thanks for the beautiful answer. The `lambda` in option 2 in my example was actually a leftover from a more complicated piece of code, but actually `reply.destroyed.connect(self.remove_from_list)`, without the `lambda` also seems to work on my system: I am not seeing the `ValueError`. Am I missing the point here? (this is based on legacy code using Python 2.7 and PyQt 5.6) – djvg May 20 '20 at 19:59
  • 1
    @djvg There may be a difference between the APIs or maybe it is caused by the python version, I have tested it with PyQt5 5.14.2 and python 3.8.2. Your method with the lambda is safer compared to the one I used in my explanation – eyllanesc May 20 '20 at 20:08