PyQt QNAM, QSaveFile Crashing issue

Wallboy

I have been having an issue with Qt (PyQT 5.7) crashing when using QNetworkAccessManager to download a file that gets saved to a QSaveFile.

I create a QSaveFile instance called downloadFile, setup my QNetworkRequest, and then call the QNAM get(). I then set a property on the QNetworkReply like so: reply.setProperty('downloadFile' downloadFile) and also connect the reply to a finished slot and readyRead slot.

However, once in the readyRead slot I try to access the QSaveFile by: self.sender().property('downloadFile'). This immediately crashes Qt without any sort of Python exception. The crash occuring in QtCore.dll according to my event viewer.

From what I can tell, the issue seems to be that a reference to the QSaveFile is not kept and Python is garbage collecting it by the time the readyRead slot is called. To test this, I created a list and first append the downloadFile to it in the same function where I create the request. The crash no longer happens and in the finished slot all i have to do is remove it from the list to clean up. This is how I have been getting around this 'bug' for awhile now (although I'm not sure if it's an actual bug or my own misunderstanding of how I'm supposed to do it in Python).

However I came up with an alternative solution that doesn't require downloadFile to be added to a 'named' container first: reply.setProperty('downloadFile', (downloadFile,))

This works! By wrapping the QSaveFile in a Tuple, it prevents the crash. And in the readyRead slot I can access it using self.sender().property('downloadFile')[0] without crashing. For the heck of it though I tried wrapping it in a list: reply.setProperty('downloadFile', [downloadFile])

But this crashes on property() access later as well. Weird. Tuple works, List doesn't.

Would love to know from any Qt/PyQt/ gurus on why these crashes occur. Why can I not simply do: reply.setProperty('downloadFile' downloadFile)?

PS. I also tried attaching downloadFile as an attribute of the request object using: request.setAttribute(QNetworkRequest.User, downloadFile). However the crash occurs later on as well when trying to retrieve downloadFile: self.sender().request().attribute(QNetworkRequest.User)

Here is a complete example demonstrating this problem in PyQt 5.7. Use the comments to switch between the working and crashing versions

import sys
from PyQt5.QtCore import QCoreApplication, QTimer, QUrl, QSaveFile, QIODevice, QObject
from PyQt5.QtNetwork import QNetworkRequest, QNetworkAccessManager


class NetworkAccessManager(QObject):

    def __init__(self):
        super().__init__()
        self.qnam = QNetworkAccessManager()
        self.outFiles = []

    def doDownload(self):

        url = QUrl('http://ipv4.download.thinkbroadband.com/10MB.zip')
        request = QNetworkRequest(url)

        outFile = QSaveFile('10MB.zip')
        outFile.open(QIODevice.WriteOnly)

        # self.outFiles.append(outFile)  # Container needed for third version of setProperty() below

        reply = self.qnam.get(request)
        reply.finished.connect(self.finishedSlot)
        reply.readyRead.connect(self.dataReadyRead)

        # Works
        reply.setProperty('downloadFile', (outFile,))

        # Crashes in property() call later in dataReadyRead. Tuple version above works, List doesn't. Why?
        # reply.setProperty('downloadFile', [outFile])

        # Crashes in property() call later in dataReadyRead, UNLESS outFile added to a container above
        # reply.setProperty('downloadFile', outFile)

    def finishedSlot(self):
        reply = self.sender()
        reply.property('downloadFile')[0].commit()

        # For third setProperty() version above w/ append line uncommented as well
        # reply.property('downloadFile').commit()
        # self.outFiles.remove(reply.property('downloadFile'))

        reply.deleteLater()
        QCoreApplication.exit()

    def dataReadyRead(self):
        reply = self.sender()
        outFile = reply.property('downloadFile')[0]

        # For third version of setProperty() above
        # outFile = reply.property('downloadFile')

        if outFile.write(reply.readAll()) == -1:
            print("An error occured writing the file")

if __name__ == '__main__':
    app = QCoreApplication(sys.argv)
    downloader = NetworkAccessManager()
    QTimer.singleShot(0, downloader.doDownload)
    sys.exit(app.exec_())

SGaist

Hi,

From a quick look, it's likely that outFile gets destroyed at the end of doDownload since it's local to that function. You could make it a member of your NetworkAccessManager class.

Wallboy

@SGaist

Right, but I thought maybe setProperty would keep a reference to it. After all "reply" is also local to that function and does not get destroyed when leaving. Though I'm sure other names get binded to it in the signal connections somewhere.

And yes, making it an attribute of the class by creating a list of outFiles as I do in the example program provided, does indeed work and is how I have been doing it until I started refactoring some of my code and wanted to figure out a better way of doing it. And the cleanest way I have found thus far is wrapping it in a tuple in the setProperty() call.

But I'm still not sure why this works, but wrapping it in a list does not. Not that it particularly matters, I'm just curious what is going on behind the scenes.

SGaist

From the C++ side, QNetworkReply is not local in the sense that you get a pointer to a heap allocated object that lifetime you must handle yourself while outFile would a stack allocated object that would be destroyed at the end of the function.