Segmentation fault for scoped references with multiple QNetworkReply objects

[Phil Thompson]

On 11/08/2024 23:44, Maurizio Berti wrote:
> I am writing a web radio player as personal project. Other than 
> accessing
> the media stream, it also access some related files (json for the 
> program
> list, related images, etc.)
> 
> Since I'm in the middle of the development, I've not dotted all i's 
> yet.
> Specifically, some QNetworkReply objects don't get deleted yet once I'm
> done with them (which I'll eventually do with deleteLater as expected, 
> but
> I'll need to find a proper way to handle that while providing a common
> interface).
> 
> Some of those replies work by simply referencing the reply itself in a
> local function.
> For example, this is a simplified version of some code that gets a 
> remote
> image:
> 
> def getRemoteImage(self, someObject):
>     # "self.nam" is the common QNetworkAccessManager
>     reply = self.nam.get(QNetworkRequest(someObject.url()))
>     def doSomeStuff():
>         pm = QPixmap()
>         if pm.loadFromData(reply.readAll()):
>             someObject.setIcon(QIcon(pm))
>     reply.finished.connect(doSomeStuff)
> 
> I know that this is not very elegant, but it's concise, and it works 
> for
> simple situations, without requiring a more comprehensive 
> implementation
> that isn't based on local context.
> 
> Well, at least, I *thought* it worked.
> After some time and different testing, I stumbled upon unexpected
> segfaults. The result of my research is that if the replies are not
> deleted, at some point the program crashes.
> What I found interesting is that what *seems* to trigger the crash is 
> the
> number of connections done to the reply, like there's a total amount of
> possible existing replies and their scoped references.
> 
> I prepared a MRE that should be able to show the problem:
> 
> from PyQt5.QtCore import *
> from PyQt5.QtWidgets import *
> from PyQt5.QtNetwork import *
> 
> class Test(QWidget):
>     def __init__(self):
>         super().__init__()
>         btn = QPushButton('Test')
>         QVBoxLayout(self).addWidget(btn)
> 
>         self.nam = QNetworkAccessManager()
>         btn.clicked.connect(btn.setEnabled)
>         btn.clicked.connect(self.download)
> 
>         self.timer = QTimer(self, timeout=self.download, interval=100)
>         self.count = 0
> 
>         base = 'https://www.google.com/search?q={}'
>         self.urls = [QUrl(base.format(i)) for i in range(100)]
> 
>     def download(self):
>         if not self.timer.isActive():
>             self.timer.start()
>         url = self.urls.pop(0)
>         reply = self.nam.get(QNetworkRequest(url))
> 
>         def test1():
>             self.count += 1
>             print(self.count, 'finished (first connect)')
>         def test2():
>             pass
>         def test3():
>             pass
>         def testWithReference():
>             print(self.count, 'finished > reference to reply:', reply)
>             print()
> 
>         reply.finished.connect(test1)
>         reply.finished.connect(test2)
>         reply.finished.connect(test3)
> 
>         reply.finished.connect(self.doSomething)
>         reply.finished.connect(testWithReference)
> 
>         if not self.urls:
>             self.timer.stop()
> 
>     def doSomething(self):
>         print(self.sender())
> 
> 
> app = QApplication([])
> test = Test()
> test.show()
> app.exec()
> 
> I'm currently able to reproduce this on Linux with PyQt 5.15.4 (Qt 
> 5.15.2)
> and PyQt/Qt 6.6.1, with Python 3.9.5.
> Interestingly enough, this doesn't happen with an old PyQt/Qt 5.7.1 and
> Python 3.4.5.
> 
> Note that I'm completely aware that avoiding replies deletion and using
> local references are not recommended approaches, yet, at least based on 
> the
> generic behavior of wrapped Qt objects and python scope management, it
> should work in theory: a similar concept could normally be easily and
> reliably applied with other QObjects, leading me to think that, 
> probably,
> at some point there is some issue in the management of references to
> objects directly generated by Qt.
> 
> I will obviously implement a more thorough deletion mechanism for the
> replies, which, based on my tests, should avoid the problem completely.
> Yet, I wanted to report this anyway, at least in order to understand 
> its
> origins (I *suppose* it's a Python problem, but I'm not 100% sure), 
> what
> causes it and know more about the underlying aspects.
> Specifically, I'd like to understand what could happen in a stress 
> case,
> with dozens of current requests (and replies) that have not been 
> completed
> yet.
> 
> Regards,
> MaurizioB

First off, I can't reproduce the problem - even if I double the number 
of URLs to 200.

However looking at the reply object using sip.dump() shows it has no 
parent wrapped object whereas the docs for QNetworkReply::manager() 
state that the manager is the reply's parent. Adding a /Transfer/ 
annotation to QNetworkManager::get() fixes this - but I have no idea if 
this has any effect on your test case.

Phil