Quote:
Originally Posted by kovidgoyal
Yeah, it prevents the QPixmap objects from being garbage collected in case they are removed from a non GUI thread. It might causes crashes if their destructors run on a non-GUI thread. Do you suspect the deadlock being there? Its a pretty simple class but you can try changing the Lock to an RLock.
|
I think I found it. It doesn't involve RLock.
The method alternate_view.render_cover is called in the render thread, not the GUI thread. With this code (the print statements), when it fails I see only the first message. I have never seen a failure if the second message appears.
Code:
...
print(f'render_cover has_cover: {has_cover}')
if has_cover:
p = QImage()
p.loadFromData(cdata, CACHE_FORMAT if cdata is tcdata else 'JPEG')
print(f'render_cover p.isNull: {p.isNull()}, cdata is tcdata:{cdata is tcdata}')
p.setDevicePixelRatio(dpr)
What this tells me is that QImage.loadFromData() isn't thread-safe and can deadlock. This is consistent with some Qt posts that say that Qt data structures should be manipulated in the GUI thread, or at least in a QThread.
I see three possible fixes:
- Build the data structures in the GUI thread before render_covers. This isn't practical because it would eliminate the advantage of the cache.
- Use a cross-thread callback function to build the QImage in the GUI thread. This should work but might have performance problems because of signal overhead.
- Use a QThread instead of a Thread. I have no idea if this would really work, and in any event is a bother to set up with queues.
I tried the second option. It seems to work, or better said it didn't hang in many (!) attempts. Performance seems acceptable, expecially given it is a one-time call per cover. I didn't look hard to see if there are any other Qt image/file operations outside the GUI thread.
A patch file is attached. I can do it as a PR if you would rather.