Crashes on opening channels and on exit (memory safety issues?)

joanbm commented 4 years ago

I am running Orion 1.6.7 on an updated Arch Linux install and I am having crashes that seem to point to some memory safety / corruption issues.

The crashes seem to happen in two situations:

When exiting the application (even after just opening it, no need to do anything specific), it consistently crashes for me with some message related to memory issues ("double free or corruption", "invalid pointer" or similar):

Sample stacktrace:

[zealcharm@SOL ~]$ gdb -ex=run --args orion
[...]
[Thread 0x7fffce46f700 (LWP 123052) exited]
[Thread 0x7fffad8ed700 (LWP 123096) exited]
[Thread 0x7fffae0ee700 (LWP 123095) exited]
double free or corruption (out)

Thread 1 "orion" received signal SIGABRT, Aborted.
0x00007ffff5b33ce5 in raise () from /usr/lib/libc.so.6
(gdb) bt
#0  0x00007ffff5b33ce5 in raise () at /usr/lib/libc.so.6
#1  0x00007ffff5b1d857 in abort () at /usr/lib/libc.so.6
#2  0x00007ffff5b772b0 in __libc_message () at /usr/lib/libc.so.6
#3  0x00007ffff5b7e74a in  () at /usr/lib/libc.so.6
#4  0x00007ffff5b805c0 in _int_free () at /usr/lib/libc.so.6
#5  0x00007ffff68c6cdf in  () at /usr/lib/libQt5Qml.so.5
#6  0x00007ffff5fc5b3c in QHashData::free_helper(void (*)(QHashData::Node*)) () at /usr/lib/libQt5Core.so.5
#7  0x00007ffff68c0a4d in QQmlEnginePrivate::~QQmlEnginePrivate() () at /usr/lib/libQt5Qml.so.5
#8  0x00007ffff694707a in QQmlApplicationEnginePrivate::~QQmlApplicationEnginePrivate() () at /usr/lib/libQt5Qml.so.5
#9  0x00007ffff6199ec2 in QObject::~QObject() () at /usr/lib/libQt5Core.so.5
#10 0x00007ffff68c5b5b in QQmlEngine::~QQmlEngine() () at /usr/lib/libQt5Qml.so.5
#11 0x000055555556febe in main ()
(gdb)

When single-clicking a channel in the Channels or Followers view (which should open the bottom information bar with the "Watch", "Videos", etc. option), on certain channels (e.g. yes on 'SypherPK' but not on 'Evelone192') and sometimes (maybe ~50% of the time) it crashes. As I said, it seems to be related to which channel I click, but I haven't found what is the actual difference between channels that determines whether it crashes or not.

Sample stacktrace:

[zealcharm@SOL ~]$ gdb -ex=run --args orion
[...]
[New Thread 0x7fffae0ee700 (LWP 122461)]
[New Thread 0x7fffad8ed700 (LWP 122462)]
[New Thread 0x7fffad03e700 (LWP 122463)]
[Detaching after fork from child process 122464]
malloc(): invalid size (unsorted)

Thread 22 "QSGRenderThread" received signal SIGABRT, Aborted.
[Switching to Thread 0x7fffad03e700 (LWP 122463)]
0x00007ffff5b33ce5 in raise () from /usr/lib/libc.so.6
(gdb) bt
#0  0x00007ffff5b33ce5 in raise () at /usr/lib/libc.so.6
#1  0x00007ffff5b1d857 in abort () at /usr/lib/libc.so.6
#2  0x00007ffff5b772b0 in __libc_message () at /usr/lib/libc.so.6
#3  0x00007ffff5b7e74a in  () at /usr/lib/libc.so.6
#4  0x00007ffff5b816b4 in _int_malloc () at /usr/lib/libc.so.6
#5  0x00007ffff5b82fb9 in malloc () at /usr/lib/libc.so.6
#6  0x00007ffff6c5121b in QImageData::create(QSize const&, QImage::Format) () at /usr/lib/libQt5Gui.so.5
#7  0x00007ffff6c5135d in QImage::QImage(QSize const&, QImage::Format) () at /usr/lib/libQt5Gui.so.5
#8  0x00007ffff6c5139a in QImage::QImage(int, int, QImage::Format) () at /usr/lib/libQt5Gui.so.5
#9  0x00007ffff6c5230c in QImage::copy(QRect const&) const () at /usr/lib/libQt5Gui.so.5
#10 0x00007ffff6c52875 in QImage::detach() () at /usr/lib/libQt5Gui.so.5
#11 0x00007ffff6c5404b in QImage::setColor(int, unsigned int) () at /usr/lib/libQt5Gui.so.5
#12 0x00007fffe9323c48 in  () at /usr/lib/libQt5XcbQpa.so.5
#13 0x00007ffff6f1e93e in QTextureGlyphCache::textureMapForGlyph(unsigned int, QFixed) const () at /usr/lib/libQt5Gui.so.5
#14 0x00007ffff6fb10ee in QOpenGLTextureGlyphCache::fillTexture(QTextureGlyphCache::Coord const&, unsigned int, QFixed) () at /usr/lib/libQt5Gui.so.5
#15 0x00007ffff6f20635 in QTextureGlyphCache::fillInPendingGlyphs() () at /usr/lib/libQt5Gui.so.5
#16 0x00007ffff7376731 in  () at /usr/lib/libQt5Quick.so.5
#17 0x00007ffff73723bb in  () at /usr/lib/libQt5Quick.so.5
#18 0x00007ffff74473be in QQuickTextNode::addGlyphs(QPointF const&, QGlyphRun const&, QColor const&, QQuickText::TextStyle, QColor const&, QSGNode*) ()
    at /usr/lib/libQt5Quick.so.5
#19 0x00007ffff744c869 in  () at /usr/lib/libQt5Quick.so.5
#20 0x00007ffff7448421 in QQuickTextNode::addTextLayout(QPointF const&, QTextLayout*, QColor const&, QQuickText::TextStyle, QColor const&, QColor const&, QColor const&, QColor const&, int, int, int, int) () at /usr/lib/libQt5Quick.so.5
#21 0x00007ffff7446c4f in QQuickText::updatePaintNode(QSGNode*, QQuickItem::UpdatePaintNodeData*) () at /usr/lib/libQt5Quick.so.5
#22 0x00007ffff73ebc78 in QQuickWindowPrivate::updateDirtyNode(QQuickItem*) () at /usr/lib/libQt5Quick.so.5
#23 0x00007ffff73ec53c in QQuickWindowPrivate::updateDirtyNodes() () at /usr/lib/libQt5Quick.so.5
#24 0x00007ffff73edc62 in QQuickWindowPrivate::syncSceneGraph() () at /usr/lib/libQt5Quick.so.5
#25 0x00007ffff738e365 in  () at /usr/lib/libQt5Quick.so.5
#26 0x00007ffff7390404 in  () at /usr/lib/libQt5Quick.so.5
#27 0x00007ffff7394817 in  () at /usr/lib/libQt5Quick.so.5
#28 0x00007ffff5f8add6 in  () at /usr/lib/libQt5Core.so.5
#29 0x00007ffff58c146f in start_thread () at /usr/lib/libpthread.so.0
#30 0x00007ffff5bf73d3 in clone () at /usr/lib/libc.so.6
(gdb)

I wish I had more information to contribute to the bug report, but unfortunately though I spent some time trying to determine the cause of both problems (which may be the same), I haven't found anything useful. Changing settings, removing configurations, etc. does not seem to do anything. Enabling debug output also doesn't print anything useful. Using both gdb and valgrind I didn't get very far as well, as most of the time I got the crashes pretty deep into Qt code.

The only thing I remember is that a few months ago using Orion 1.6.6 I did not get those issues, but I'm not sure what could have actually caused the regression. Just posting there in case someone also has this problem and has some more information about it.

joanbm commented 4 years ago

I managed to build 1.6.6 again and found that:

The 'crash when clicking channels' issue is also there. Maybe this broke due to Qt or some other 3rd party upgrade.
The 'crash at exit' doesn't happen there. Running git bisect one reaches:

889deebcea8d45a5cca00a6a22397bb5b3ab4ef1 is the first bad commit
commit 889deebcea8d45a5cca00a6a22397bb5b3ab4ef1
Author: mrgreywater <mr.greywater@googlemail.com>
Date:   Sun Nov 18 21:28:34 2018 +0100

    refactor settings, choose player settings at runtime

 appveyor.yml                   |  41 ++----
 orion.pro                      |   5 +-
 src/main.cpp                   |  66 +++++----
 src/model/channelmanager.cpp   |  10 +-
 src/model/settingsmanager.cpp  | 266 ++++++++++++++---------------------
 src/model/settingsmanager.h    | 100 +++++++++----
 src/model/vodmanager.cpp       |   3 +-
 src/network/networkmanager.cpp |   1 +
 src/qml/MpvBackend.qml         |  17 ++-
 src/qml/OptionsView.qml        | 311 ++++++++++++++++++++++++++---------------
 src/qml/PlayerView.qml         |  50 ++-----
 11 files changed, 465 insertions(+), 405 deletions(-)

joanbm commented 4 years ago

I figured most of it out.

For the crash when exiting, in multiple commits between 1.6.6 and 1.6.7, many objects that the application creates but are owned/released by Qt were converted from heap allocations to stack allocations. This causes Qt to attempt to release them at exit, which causes the crash.

Fix:

diff --git a/src/model/imageprovider.cpp b/src/model/imageprovider.cpp
index f9a24d2..3810dbb 100644
--- a/src/model/imageprovider.cpp
+++ b/src/model/imageprovider.cpp
@@ -26,7 +26,8 @@
 const int ImageProvider::MSEC_PER_DOWNLOAD = 16; // ~ 256kbit/sec for 2k images

 ImageProvider::ImageProvider(const QString imageProviderName, const QString extension, const QString cacheDirName) : QObject(),
-    _cacheProvider(this), _imageProviderName(imageProviderName), _extension(extension) {
+    _imageProviderName(imageProviderName), _extension(extension) {
+    _cacheProvider = new CachedImageProvider(this);

     activeDownloadCount = 0;

@@ -158,7 +159,7 @@ void ImageProvider::loadImageFile(QString emoteKey, QString filename) {
 }

 QQmlImageProviderBase * ImageProvider::getQMLImageProvider() {
-    return &_cacheProvider;
+    return _cacheProvider;
 }

 bool ImageProvider::downloadsInProgress() const {
diff --git a/src/model/imageprovider.h b/src/model/imageprovider.h
index d398ee7..fab4271 100644
--- a/src/model/imageprovider.h
+++ b/src/model/imageprovider.h
@@ -100,7 +100,7 @@ private:
     QNetworkAccessManager _manager;

     friend class CachedImageProvider;
-    CachedImageProvider _cacheProvider;
+    CachedImageProvider *_cacheProvider;

     QHash<QString, QImage> _imageTable;
     QString _imageProviderName;
diff --git a/src/model/settingsmanager.cpp b/src/model/settingsmanager.cpp
index 9f4bfd7..4112ecc 100644
--- a/src/model/settingsmanager.cpp
+++ b/src/model/settingsmanager.cpp
@@ -12,8 +12,10 @@ SettingsManager::SettingsManager(QObject *parent) :

 SettingsManager *SettingsManager::getInstance()
 {
-    static SettingsManager instance;
-    return &instance;
+    static SettingsManager *instance = nullptr;
+    if (instance == nullptr)
+        instance = new SettingsManager();
+    return instance;
 }

 void SettingsManager::load()
diff --git a/src/model/vodmanager.cpp b/src/model/vodmanager.cpp
index 958aa2a..a39edbe 100644
--- a/src/model/vodmanager.cpp
+++ b/src/model/vodmanager.cpp
@@ -41,15 +41,13 @@ VodManager::VodManager(QObject *parent) :
     settings.endArray();

     emit modelChanged();
-
-    std::atexit([](){
-       VodManager::getInstance()->saveSettings();
-    });
 }

 VodManager *VodManager::getInstance() {
-    static VodManager instance;
-    return &instance;
+    static VodManager *instance = nullptr;
+    if (instance == nullptr)
+        instance = new VodManager();
+    return instance;
 }

 VodManager::~VodManager()

I did a quick test by counting ctor/dtor calls and even with this change all of CachedImageProvider, VodManager and SettingsManager are properly released at exit, there's no leak. Unless I'm missing something the call to std::atexit() can be safely removed since the settings are already saved at VodManager::~VodManager which happens at app. exit AFAICT.

For the crash when clicking channels I found that when a channel's description has emoji characters, it crashes. At first glance this looks like an upstream issue (Qt or similar). This can be used as a workaround:

diff --git a/src/qml/util.js b/src/qml/util.js
index 75f9a6a..7358805 100644
--- a/src/qml/util.js
+++ b/src/qml/util.js
@@ -19,7 +19,7 @@ function copyChannel(channel) {
         _id: channel._id,
         name: channel.name,
         title: channel.title,
-        info: channel.info,
+        info: channel.info.replace(/[^\x00-\x7F]/g, ""),
         logo: channel.logo,
         preview: channel.preview,
         game: channel.game,

As an extra, for some reason hardware acceleration is working very slowly for me now, on an Intel iGPU I get ~5fps on 1080p60 videos with vaapi-copy. I think this is also an upstream issue (Intel drivers? mesa?) but I found that using the "gpu" backend (instead of the "libmpv" default backend) makes it work again. This makes the video play in a separate window as a side effect so it's not really something mergeable at this moment. Patch:

diff --git a/src/player/mpvobject.cpp b/src/player/mpvobject.cpp
index 31cdf73..f8ccfab 100644
--- a/src/player/mpvobject.cpp
+++ b/src/player/mpvobject.cpp
@@ -128,6 +128,8 @@ MpvObject::MpvObject(QQuickItem * parent)

 #ifdef USE_OPENGL_CB
     mpv_set_option_string(mpv, "vo", "opengl-cb");
+#else
+    mpv_set_option_string(mpv, "vo", "gpu");
 #endif

     if (mpv_initialize(mpv) < 0)

This makes Orion usable again for me.

joanbm commented 4 years ago

The emoji crash bug is a bug on Qt: [QTBUG-82311] Crash/Assert rendering text with emoji when style is set

alamminsalo / orion

Crashes on opening channels and on exit (memory safety issues?) #302