Bug 1141444

(In reply to Jiri Slaby from comment #0)
> 1) it allocates 20T of virtual memory on startup. What is the reason for
> that?

It's perhaps the memory shadow of the ASAN. So that is likely OK.

Created attachment 810435 [details]
clementine ran with valgrind --leak-check=full

The reason for -fsanitize=address was an attempt to address the many issues clementine has with memory. For some reason that I can't put my finger on yet it behaves differently on different hardware. There is one definite issue with the enabling of the "save statistics in file tags when possible"  see bug 1138261
I Leap:15.0 and 1 executing "clementine --help" intermittently cause a double free crash if the version is 1.3.1 or greater.
The other current bug is Bug 1137785

I would appreciate some help.

Created attachment 810464 [details]
delete the allocated mem

I don't have the stats-to-tags ticked in the settings.

For a start, this patch deletes the allocated memory in respective destructors. Some of the largest leaks indeed dismissed. But there are more, perhaps. ASAN does not do a very nice reporting. valgrind revealed the sources better.

Did they mean to use a shared pointer or they don't care about memory allocations at all?

Created attachment 810465 [details]
delete the allocated mem v2

Created attachment 810466 [details]
delete the allocated mem

Maybe the third time, I will attach the correct one.

(In reply to Jiri Slaby from comment #6)
> Created attachment 810466 [details]
> delete the allocated mem
> 
> Maybe the third time, I will attach the correct one.

Thanks, I'm no longer using asan in home:plater/clementine the version there is the current one it includes your patch and the latest qt5 branch git.

Created attachment 810549 [details]
delete the allocated mem v3

sqlite connections are leaked during backup too as nullptrs are passed to the EXIT block, so nothing is freed. Pass by reference instead...

I've updated the patch, one of the Bug 1137785 issues has been fixed in the latest git snapshot so I'll be submitting what I have to Factory.

Well, with ASAN disabled and the patch, clementine is now at 223 MB and doesn't grow much. I still see leaks in plugins, leaking Network objects. It's 84 files to edit (most of the plugins), so it will take some time.

For example ArtistBiography or Musicbrainzcover:
> 1,400 (112 direct, 1,288 indirect) bytes in 1 blocks are definitely lost in loss record 10,967 of 11,287
>    at 0x4836DEF: operator new(unsigned long) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
>    by 0x50F9AF1: QObject::QObject(QObject*) (qobject.cpp:807)
>    by 0x5B309CA: QNetworkAccessCache::QNetworkAccessCache() (qnetworkaccesscache.cpp:105)
>    by 0x5B2AFDF: QNetworkAccessManagerPrivate (qnetworkaccessmanager_p.h:102)
>    by 0x5B2AFDF: QNetworkAccessManager::QNetworkAccessManager(QObject*) (qnetworkaccessmanager.cpp:469)
>    by 0x7A057B: NetworkAccessManager::NetworkAccessManager(QObject*) (network.cpp:87)
>    by 0x44618D: ArtistBiography::ArtistBiography() (artistbiography.cpp:55)
>    by 0x50C982: ArtistInfoView::ArtistInfoView(QWidget*) (artistinfoview.cpp:29)
>    by 0x66278E: MainWindow::MainWindow(Application*, SystemTrayIcon*, OSD*, CommandlineOptions const&, QWidget*) (mainwindow.cpp:187)
>    by 0x3B8A0C: main (main.cpp:456)
>
> 1,816 (16 direct, 1,800 indirect) bytes in 1 blocks are definitely lost in loss record 11,000 of 11,287
>    at 0x4836DEF: operator new(unsigned long) (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
>    by 0x7A0590: NetworkAccessManager::NetworkAccessManager(QObject*) (network.cpp:88)
>    by 0x77827C: MusicbrainzCoverProvider::MusicbrainzCoverProvider(QObject*) (musicbrainzcoverprovider.cpp:41)
>    by 0x7C0515: std::_Function_handler<CoverProviders* (), ApplicationImpl::ApplicationImpl(Application*)::{lambda()#7}>::_M_invoke(std::_Any_data const&) (application.cpp:101)
>    by 0x7BEAAE: UnknownInlinedFun (std_function.h:690)
>    by 0x7BEAAE: UnknownInlinedFun (lazy.h:57)
>    by 0x7BEAAE: UnknownInlinedFun (lazy.h:55)
>    by 0x7BEAAE: UnknownInlinedFun (lazy.h:36)
>    by 0x7BEAAE: Application::cover_providers() const (application.cpp:245)
>    by 0x4F4F5E: AlbumCoverChoiceController::SetApplication(Application*) (albumcoverchoicecontroller.cpp:90)
>    by 0x63B6A9: NowPlayingWidget::SetApplication(Application*) (nowplayingwidget.cpp:184)
>    by 0x6672A7: MainWindow::MainWindow(Application*, SystemTrayIcon*, OSD*, CommandlineOptions const&, QWidget*) (mainwindow.cpp:244)
>    by 0x3B8A0C: main (main.cpp:456)

There seems to be a repetitive problem with fontrendering (fontconfig), which I will into later. This perhaps makes clementine memory footprint to grow.

Let me know if you want me to create PR on these or you plan to do it yourself. I don't want to take credit for it.

StaticInit() in database.cpp is done for each connection, isn't that supposed to be done once?

I don't understand what all the QList<QPair<QString, QString> leaks are about.

Created attachment 811247 [details]
delete the allocated mem v4

With this patch, after 4 days of uptime, clementine is at ~ 360 MB, which seems to be an improvement. It makes most of QNetworkAccessManager's static. Some of them are made unique_ptr and some are deleted, depending on constructor.

Each thread creates its own database connection, it seems.

I can crate a PR, but it will take some time before I process it into some upstreamable state. So feel free to take this and only note me as an author.

Created attachment 811251 [details]
clementine ran with valgrind --leak-check=full

(In reply to Jonas Kvinge from comment #12)
> I don't understand what all the QList<QPair<QString, QString> leaks are
> about.

Better to look into the valgrind report, not the ASAN's. They are UltimateLyricsReader leaks:

> 992 bytes in 31 blocks are possibly lost in loss record 11,742 of 12,222
>    at 0x483677F: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
>    by 0x4F5123F: QListData::detach_grow(int*, int) (qlist.cpp:96)
>    by 0x67C2C5: UnknownInlinedFun (qlist.h:775)
>    by 0x67C2C5: QList<QList<QPair<QString, QString> > >::append(QList<QPair<QString, QString> > const&) (qlist.h:601)
>    by 0x67C682: UnknownInlinedFun (qlist.h:404)
>    by 0x67C682: UnknownInlinedFun (ultimatelyricsprovider.h:52)
>    by 0x67C682: UltimateLyricsReader::ParseProvider(QXmlStreamReader*) const (ultimatelyricsreader.cpp:79)
>    by 0x67C9C2: UltimateLyricsReader::ParseDevice(QIODevice*) const (ultimatelyricsreader.cpp:51)
>    by 0x67CB0D: UltimateLyricsReader::Parse(QString const&) const (ultimatelyricsreader.cpp:39)
>    by 0x67CF68: non-virtual thunk to QtConcurrent::RunFunctionTask<QList<SongInfoProvider*> >::run() (qtconcurrentrunbase.h:119)
>    by 0x4EFF431: QThreadPoolThread::run() (qthreadpool.cpp:99)
>    by 0x4EFC111: QThreadPrivate::start(void*) (qthread_unix.cpp:360)
>    by 0x78D8FA9: start_thread (pthread_create.c:486)
>    by 0x568373E: clone (clone.S:95)

I am not sure if UltimateLyricsReader is ever deleted. Note it looks to be allocated with every song change. Also, even if it was released, I don't know, if it would delete the parser data. (I don't have time to look into it.)

I've updated delete_the_allocated_memory_in_respective_destructors.patch
in home:plater I also plan to submit to Leap:15.0 and 1 clementine-1.3.1.

This is an autogenerated message for OBS integration:
This bug (1141444) was mentioned in
https://build.opensuse.org/request/show/718242 15.0+15.1+Backports:SLE-15 / clementine

The leak caused by the BOOST_SCOPE_EXIT mistake in database is a bad one, and obviously a leak. Also the QNetworkAccessManager in tagreader. But QNetworkAccessManager used everywhere else shouldn't leak.

new QNetworkAccessManager(this) is correct, because Qt objects that inherit the parent will be deleted when the parent is deleted (memory is managed by Qt).

Same with NetworkAccessManager: https://github.com/clementine-player/Clementine/blob/master/src/core/network.cpp#L87

Afer update to Build 20190723 of tumbleweed, the 20Tb problem has been fixed. But the biggest issue with memory leak is when you turn the projectM visualizer on. In just 5 seconds usage, the memory consumptions increases to few gigabytes and the swap usage keeps on increasing till the machine runs out of memory. The load increases and the machine becomes unresponsive.

top - 11:28:23 up 20 min,  5 users,  load average: 2.55, 1.83, 1.35
Tasks: 338 total,   4 running, 333 sleeping,   0 stopped,   1 zombie
%Cpu(s): 24.0 us,  2.7 sy,  0.0 ni, 68.8 id,  0.9 wa,  0.0 hi,  3.7 si,  0.0 st
MiB Mem : 7929.961 total,  129.785 free, 3039.844 used, 4760.332 buff/cache
MiB Swap: 8195.996 total, 8165.746 free,   30.250 used. 4558.633 avail Mem

  PID USER      PR  NI    VIRT    RES    SHR S  %CPU  %MEM     TIME+ COMMAND
28548 mbhangui  20   0 3831228 1.379g 139216 R 106.6 17.81   0:27.68 clementine

At the moment I am using clementine by copying the executable from a FC30 build
I also have to copy the following shared libraries to have a working music player on tumbleweed

libcdio.so.18.0.0
libprojectM.so.2.1.0
libprotobuf.so.17.0.0
libQxtCore-qt5.so.0.7.0
libQt5Solutions_LockedFile-2.4.so.1.0.0
libcryptopp.so.8.2.0
libQxtWidgets-qt5.so.0.7.0
libQt5Solutions_IOCompressor-2.3.so.1.0.0
libQt5Solutions_SingleCoreApplication-2.6.so.1.0.0
libQt5Solutions_SingleApplication-2.6.so.1.0.0
libsha2.so.1.0.1

(In reply to Jonas Kvinge from comment #17)
> The leak caused by the BOOST_SCOPE_EXIT mistake in database is a bad one,
> and obviously a leak. Also the QNetworkAccessManager in tagreader. But
> QNetworkAccessManager used everywhere else shouldn't leak.
> 
> new QNetworkAccessManager(this) is correct, because Qt objects that inherit
> the parent will be deleted when the parent is deleted (memory is managed by
> Qt).

OK, thanks for review. So could you please pick only the good pieces from the patch, merge it and I will retest when we merge it. Maybe the problem is that NetworkAccessManager _users_ are not freed upon exit... Like it looks like song provider list is not.

I can confirm that qt5 branch even with the v4 patch has a serious memory problem when visualizations are started and is unusable. Clementine-1.3.1 works.

To add to this my Tumbleweed laptop which has only had clementine qt branch installed doesn't have any visualizations at all.
Could this be because the visualization downloads are broken?
The song info and album art works.

I did some additional test by running clementine on foreground (both the FC30 version and the openSUSE version). This is my observation when the visualizer is turned on

Using the FC30 build, I just get the following message on the screen when I turn visualizer on

[projectM] Allocating idle preset...
[PresetFactory] path is Geiss & Sperl - Feedback (projectM idle HDR mix).milk
[PresetFactory] url is idle://Geiss & Sperl - Feedback (projectM idle HDR mix).milk
[PresetFactory] path is r/share/projectM/presets/Hexcollie, geiss, Flexi, yin n Phat - Pinwheel.milk
[PresetFactory] url is /usr/share/projectM/presets/Hexcollie, geiss, Flexi, yin n Phat - Pinwheel.milk

On the openSUSE build, the bottom three messages keep repeating infinitely

[PresetFactory] url is idle://Geiss & Sperl - Feedback (projectM idle HDR mix).milk
No Textures Loaded from /usr/share/projectM/textures
[PresetFactory] url is /usr/share/projectM/presets/LuxXx - Subtle HipHopFlake.milk
Successfull compilation of Warp
Successfull compilation of Comp
No Textures Loaded from /usr/share/projectM/textures
Successfull compilation of Warp
Successfull compilation of Comp
No Textures Loaded from /usr/share/projectM/textures
..
..
Successfull compilation of Warp
Successfull compilation of Comp
No Textures Loaded from /usr/share/projectM/textures

The lack of visualizations is caused by a lack of projectM-data, I've made clementine recommend it.

My poor laptop struggles to play songs at all with visualizations.

(In reply to Dave Plater from comment #24)
> My poor laptop struggles to play songs at all with visualizations.

There are two problems with the visualization. 

1. Continous memory allocation
2. The rendering is done by the software (probably this will require a separate bug report). Your laptop will suffering because of this. Since the rendering is done by clementine, the cpu utilization of clementine hits the roof. In the original build around > 6 months back, the rendering was probably done by the driver. I'm pasting top output for FC30 build and the top output of the opensuse clementine build. In the FC30 build, when I turn on the visualization, it is 'X' that starts consuming a bit more CPU. In the opensuse build, the utilization of 'X' doesn't change at all, but CPU utilization of clementine shoots up.

Top output of FC30 clementine with visualizer turned on
-----------------------------------------------------------
top - 13:13:49 up 26 min,  2 users,  load average: 0.72, 0.57, 0.46
Tasks: 315 total,   3 running, 311 sleeping,   0 stopped,   1 zombie
%Cpu(s):  8.3 us,  1.1 sy,  0.0 ni, 90.4 id,  0.0 wa,  0.0 hi,  0.2 si,  0.0 st
MiB Mem : 7921.395 total, 3520.688 free, 1511.516 used, 2889.191 buff/cache
MiB Swap: 8195.996 total, 8195.996 free,    0.000 used. 6148.215 avail Mem

  PID USER      PR  NI    VIRT    RES    SHR S  %CPU  %MEM     TIME+ COMMAND
 2350 root      20   0  205000  69368  41884 S 30.56 0.855   1:05.96 X
 8991 mbhangui  20   0 2661292 217756 130312 S 26.91 2.685   0:41.83 clementineO

Top output of opensuse clementine with visualizer turned on
-----------------------------------------------------------
top - 16:42:25 up  3:54,  5 users,  load average: 0.81, 0.73, 0.67
Tasks: 318 total,   2 running, 315 sleeping,   0 stopped,   1 zombie
%Cpu(s): 10.2 us,  2.0 sy,  0.0 ni, 77.7 id,  9.7 wa,  0.0 hi,  0.4 si,  0.0 st
MiB Mem : 7921.395 total,  978.055 free, 5192.641 used, 1750.699 buff/cache
MiB Swap: 8195.996 total, 8178.984 free,   17.012 used. 2360.668 avail Mem

  PID USER      PR  NI    VIRT    RES    SHR S  %CPU  %MEM     TIME+ COMMAND
11298 mbhangui  20   0 6344332 3.744g 123048 R 94.37 48.40   1:05.19 clementine
 2350 root      20   0  229568  92216  44568 S 3.642 1.137  66:23.70 X

(In reply to Manvendra Bhangui from comment #25)
> (In reply to Dave Plater from comment #24)
> > My poor laptop struggles to play songs at all with visualizations.
> 
> There are two problems with the visualization. 
> 
> 1. Continous memory allocation
> 2. The rendering is done by the software (probably this will require a
> separate bug report). Your laptop will suffering because of this. Since the
> rendering is done by clementine, the cpu utilization of clementine hits the
> roof. In the original build around > 6 months back, the rendering was
> probably done by the driver. I'm pasting top output for FC30 build and the
> top output of the opensuse clementine build. In the FC30 build, when I turn
> on the visualization, it is 'X' that starts consuming a bit more CPU. In the
> opensuse build, the utilization of 'X' doesn't change at all, but CPU
> utilization of clementine shoots up.
> 
> Top output of FC30 clementine with visualizer turned on
> -----------------------------------------------------------
> top - 13:13:49 up 26 min,  2 users,  load average: 0.72, 0.57, 0.46
> Tasks: 315 total,   3 running, 311 sleeping,   0 stopped,   1 zombie
> %Cpu(s):  8.3 us,  1.1 sy,  0.0 ni, 90.4 id,  0.0 wa,  0.0 hi,  0.2 si,  0.0
> st
> MiB Mem : 7921.395 total, 3520.688 free, 1511.516 used, 2889.191 buff/cache
> MiB Swap: 8195.996 total, 8195.996 free,    0.000 used. 6148.215 avail Mem
> 
>   PID USER      PR  NI    VIRT    RES    SHR S  %CPU  %MEM     TIME+ COMMAND
>  2350 root      20   0  205000  69368  41884 S 30.56 0.855   1:05.96 X
>  8991 mbhangui  20   0 2661292 217756 130312 S 26.91 2.685   0:41.83
> clementineO
> 
> Top output of opensuse clementine with visualizer turned on
> -----------------------------------------------------------
> top - 16:42:25 up  3:54,  5 users,  load average: 0.81, 0.73, 0.67
> Tasks: 318 total,   2 running, 315 sleeping,   0 stopped,   1 zombie
> %Cpu(s): 10.2 us,  2.0 sy,  0.0 ni, 77.7 id,  9.7 wa,  0.0 hi,  0.4 si,  0.0
> st
> MiB Mem : 7921.395 total,  978.055 free, 5192.641 used, 1750.699 buff/cache
> MiB Swap: 8195.996 total, 8178.984 free,   17.012 used. 2360.668 avail Mem
> 
>   PID USER      PR  NI    VIRT    RES    SHR S  %CPU  %MEM     TIME+ COMMAND
> 11298 mbhangui  20   0 6344332 3.744g 123048 R 94.37 48.40   1:05.19
> clementine
>  2350 root      20   0  229568  92216  44568 S 3.642 1.137  66:23.70 X

If clementine 1.3.1 can do it then the qt5 branch should also.
I think you should be patient with this bug, we are aware that visualizations are a memory hog.

(In reply to Dave Plater from comment #26)
> If clementine 1.3.1 can do it then the qt5 branch should also.
> I think you should be patient with this bug, we are aware that
> visualizations are a memory hog.

indeed. You guys are doing a good job. if there is a way I can help in testing and reduce the load on you guys, I will gladly do so. I know C (but not C++). I will give a try to clone the git repo and build clementine on my PC. I find strawberry a better player. I just wish strawberry had the visualizations and the ability to set ratings.

https://patch-diff.githubusercontent.com/raw/clementine-player/Clementine/pull/6375.patch

I checked to see if there were any leaks in Clementine related to visualisations use and it seem to be coming from projectm if there is any. I used leaksanitizer. 
There are no new's in the code other than in the constructors and in Init's, there is some buffer allocated through gstreamer but that's unref'ed properly.

About the stuff in songinfo, there is a lot of memory allocated that isn't free-ed on exit, but there isn't anything that's done for each song change.
One of them is here:
https://github.com/clementine-player/Clementine/blob/master/src/songinfo/ultimatelyricsreader.cpp#L66
But that's actually done just on startup for each provider (but there are like 10-20 or something).

(In reply to Jonas Kvinge from comment #29)
> I checked to see if there were any leaks in Clementine related to
> visualisations use and it seem to be coming from projectm if there is any. I
> used leaksanitizer. 

I built stock clementine-1.3.1 against Tumbleweeds projectM, libprojectM.so.3 and it freezes my machine in about 30seconds. When the same build is performed against Leap's libprojectM.so.2 the resulting clementine plays visualisations with no problem at all, in fact I can play visualisations with both Firefox and Thunderbird open which means it uses very little memory.

I'm going to open a separate bug for projectM
Opened Bug 1143132

openSUSE-RU-2019:1998-1: An update that has two recommended fixes can now be installed.

Category: recommended (moderate)
Bug References: 1138261,1141444
CVE References: 
Sources used:
openSUSE Leap 15.1 (src):    clementine-1.3.1-lp151.3.6.1
openSUSE Leap 15.0 (src):    clementine-1.3.1-lp150.2.6.1
openSUSE Backports SLE-15 (src):    clementine-1.3.1-bp150.2.6.1

openSUSE-RU-2019:2026-1: An update that has two recommended fixes can now be installed.

Category: recommended (moderate)
Bug References: 1138261,1141444
CVE References: 
Sources used:
openSUSE Backports SLE-15-SP1 (src):    clementine-1.3.1-bp151.4.6.1

The patch delete_the_allocated_memory_in_respective_destructors.patch is now fully incorporated in clementine qt5 branch git #664c5a31f9cf519af5a4c3adcb30dbbc73d037d6 from 2019 October 04
Sent to Factory via sr#738088
Can I close this bug?

(In reply to Dave Plater from comment #33)
> The patch delete_the_allocated_memory_in_respective_destructors.patch is now
> fully incorporated in clementine qt5 branch git
> #664c5a31f9cf519af5a4c3adcb30dbbc73d037d6 from 2019 October 04
> Sent to Factory via sr#738088
> Can I close this bug?

I can confirm that the issue is fixed for me

This appears to have been resolved