YACReader segfaults when switching comics too fast (unarr only?)

[selmf]

When switching between comics too fast YACReader sometimes tries to access a page index that is out of range for the current comic, resulting in a segfault and crash.

Backtrace:

#0  0x00007ffff534aef5 in raise () at /usr/lib/libc.so.6
#1  0x00007ffff5334862 in abort () at /usr/lib/libc.so.6
#2  0x00007ffff74a6dcf in __interceptor_abort(int) (fake=-10808) at /build/gcc/src/gcc/libsanitizer/tsan/tsan_interceptors_posix.cpp:1772
#3  0x00007ffff59509ac in  () at /usr/lib/libQt5Core.so.5
#4  0x00007ffff594fdb6 in  () at /usr/lib/libQt5Core.so.5
#5  0x00005555555928c2 in QVector<bool>::operator[](int) (this=0x7b1c00033200, i=29) at /usr/include/qt/QtCore/qvector.h:462
#6  0x00005555555e2550 in Render::fillBuffer() (this=0x7b1c000331b0) at render.cpp:984
#7  0x00005555555ddfb1 in Render::render() (this=0x7b1c000331b0) at render.cpp:436
#8  0x00005555555e008d in Render::update() (this=0x7b1c000331b0) at render.cpp:647
#9  0x00005555555e1ac8 in Render::pageRawDataReady(int) (this=0x7b1c000331b0, page=28) at render.cpp:879
#10 0x000055555564b2b6 in Render::qt_static_metacall(QObject*, QMetaObject::Call, int, void**)
    (_o=0x7b1c000331b0, _c=QMetaObject::InvokeMetaMethod, _id=30, _a=0x7b1c00069898) at moc_render.cpp:426
#11 0x00007ffff5ba2532 in QObject::event(QEvent*) () at /usr/lib/libQt5Core.so.5
#12 0x00007ffff68d0752 in QApplicationPrivate::notify_helper(QObject*, QEvent*) () at /usr/lib/libQt5Widgets.so.5
#13 0x00007ffff5b75a2a in QCoreApplication::notifyInternal2(QObject*, QEvent*) () at /usr/lib/libQt5Core.so.5
#14 0x00007ffff5b78523 in QCoreApplicationPrivate::sendPostedEvents(QObject*, int, QThreadData*) () at /usr/lib/libQt5Core.so.5
#15 0x00007ffff5bcf054 in  () at /usr/lib/libQt5Core.so.5
#16 0x00007ffff4326b54 in g_main_context_dispatch () at /usr/lib/libglib-2.0.so.0
#17 0x00007ffff437abf1 in  () at /usr/lib/libglib-2.0.so.0
#18 0x00007ffff4325381 in g_main_context_iteration () at /usr/lib/libglib-2.0.so.0
#19 0x00007ffff5bce691 in QEventDispatcherGlib::processEvents(QFlags<QEventLoop::ProcessEventsFlag>) () at /usr/lib/libQt5Core.so.5
#20 0x00007ffff5b743ac in QEventLoop::exec(QFlags<QEventLoop::ProcessEventsFlag>) () at /usr/lib/libQt5Core.so.5
#21 0x00007ffff5b7c844 in QCoreApplication::exec() () at /usr/lib/libQt5Core.so.5
#22 0x00005555555873e9 in main(int, char**) (argc=1, argv=0x7fffffffdfb8) at main.cpp:185

This is likely the result of an incomplete cleanup when the previous comic did not finish loading before being removed.

@vedgy , I have reason to believe that #202 would fix this issue. Could you please verify this for me and if it turns out to be the case, create a stripped down version of the fix with minimal impact on the current code structure? We will likely need to apply this fix to both master and develop and #202 has too many changes to safely do that.

[vedgy]

No, #202 is highly unlikely to fix this crash. It just quits the idle thread when it finishes doing useful work. And it does so in YACReader Server. The Render and Comic code is touched only to reduce code duplication and to do some cleanup.

I cannot reproduce your crash. It is likely caused by some other thread safety bug. I keep finding bugs in much of YACReader's multithreading code. I think in this code base threads are 1) overused and 2) not treated with sufficient care.

[selmf]

No, these are neither overused nor treated with insufficient care, these are legacy code and you should not assume you know better how to treat these than people working on them and with them for years. Any change on thread related code needs to be carefully tested and that includes the "bugs you keep finding".

[selmf]

A closer inspection using ThreadSanitizer reveals this:

WARNING: ThreadSanitizer: data race (pid=21452)
  Read of size 4 at 0x7b6000191004 by main thread:
    #0 QVector<QByteArray>::at(int) const /usr/include/qt/QtCore/qvector.h:454 (YACReader+0x7b14a)
    #1 Render::render() /home/herr_k/builds/yacreader-git/yacreader/YACReader/render.cpp:409 (YACReader+0x72ac2)
    #2 Render::update() /home/herr_k/builds/yacreader-git/yacreader/YACReader/render.cpp:647 (YACReader+0x74e66)
    #3 Render::pageRawDataReady(int) /home/herr_k/builds/yacreader-git/yacreader/YACReader/render.cpp:880 (YACReader+0x768b5)
    #4 Render::qt_static_metacall(QObject*, QMetaObject::Call, int, void**) /home/herr_k/builds/yacreader-git/yacreader/YACReader/moc_render.cpp:426 (YACReader+0xc558a)

I'm still not sure which condition triggers the race but it seems some variables are not properly cleared or updated in some cases.