freeciv21 icon indicating copy to clipboard operation
freeciv21 copied to clipboard

Memory leaks in client

Open psampathkumar opened this issue 2 years ago • 2 comments

Describe the bug We have a bunch of memory leaks in client found using -fsanitize=leak which we need to fix.

To Reproduce

  • Compile using -fsanitize=leak flag.
  • Let an automated game run. Settings for the automated game
set gamelog 30
set aifill 4
set endt 500
set timeout 1
set autosave "INTERRUPT" 
set minp 0
set gameseed 42
set mapseed 42
start
  • Connect using a client and observe and then exit the client.

Expected behavior Ideally, we should not have any leaks.

Platform and version (please complete the following information):

  • OS:Linux
  • Freeciv21 version: v3.0-beta.1
  • Ruleset/Longturn game (if applicable): classic

Additional context Information on the leaks. See attached log file. log.txt

psampathkumar avatar Mar 22 '22 17:03 psampathkumar

Using this issue to document overall progress.

Improved

I don't think I caught all 92 leaks but at least 4 per tileset load.

Direct leak of 2944 byte(s) in 92 object(s) allocated from:
    #0 0x7f82589e1811 in operator new(unsigned long) /usr/src/debug/gcc/libsanitizer/asan/asan_new_delete.cpp:99
    #1 0x5638c187106d in crop_sprite(QPixmap const*, int, int, int, int, QPixmap const*, int, int) /home/pranav/freeciv/freeciv21/client/gui-qt/sprite.cpp:76
    #2 0x5638c256fdaf  (/home/pranav/freeciv/freeciv21/build/freeciv21-client+0x43a6daf)

Indirect leak of 590968 byte(s) in 94 object(s) allocated from:
    #0 0x7f82589dfdd9 in __interceptor_malloc /usr/src/debug/gcc/libsanitizer/asan/asan_malloc_linux.cpp:145
    #1 0x7f8257889d87 in QImageData::create(QSize const&, QImage::Format) (/usr/lib/libQt5Gui.so.5+0x184d87)

Indirect leak of 144992 byte(s) in 92 object(s) allocated from:
    #0 0x7f82589e1811 in operator new(unsigned long) /usr/src/debug/gcc/libsanitizer/asan/asan_new_delete.cpp:99
    #1 0x7f8257aa8bc8 in QRasterPaintEngine::QRasterPaintEngine(QPaintDevice*) (/usr/lib/libQt5Gui.so.5+0x3a3bc8)

Fixed

Direct leak of 122 byte(s) in 12 object(s) allocated from:
    #0 0x7f82589e1991 in operator new[](unsigned long) /usr/src/debug/gcc/libsanitizer/asan/asan_new_delete.cpp:102
    #1 0x5638c1876d87 in qtg_get_useable_themes_in_directory(QString&, int*) /home/pranav/freeciv/freeciv21/client/gui-qt/themes.cpp:222

Direct leak of 8 byte(s) in 1 object(s) allocated from:
    #0 0x7f82589e1811 in operator new(unsigned long) /usr/src/debug/gcc/libsanitizer/asan/asan_new_delete.cpp:99
    #1 0x5638c19b6893 in civstatus::civstatus(QWidget*) /home/pranav/freeciv/freeciv21/client/gui-qt/civstatus.cpp:43
    #2 0x5638c165a6cc in pageGame::pageGame(QWidget*) /home/pranav/freeciv/freeciv21/client/gui-qt/page_game.cpp:127
    #3 0x5638c1316da7 in fc_client::fc_client() /home/pranav/freeciv/freeciv21/client/gui-qt/fc_client.cpp:110

Didn't dare to look

(Not enough info to nail it down.)

Direct leak of 296 byte(s) in 24 object(s) allocated from:
    #0 0x7f82589e1991 in operator new[](unsigned long) /usr/src/debug/gcc/libsanitizer/asan/asan_new_delete.cpp:102
    #1 0x5638c235de21 in secfile_lookup_str_vec(section_file const*, unsigned long*, char const*, ...) /home/pranav/freeciv/freeciv21/utility/registry_ini.cpp:2152

Direct leak of 16 byte(s) in 8 object(s) allocated from:
    #0 0x7f82589e1991 in operator new[](unsigned long) /usr/src/debug/gcc/libsanitizer/asan/asan_new_delete.cpp:102
    #1 0x7f8257313cec in qstrdup(char const*) (/usr/lib/libQt5Core.so.5+0x130cec)

Likely spurious

The rest. Check that you have some Qt suppression rules, many of them are in Qt, for instance the many instances of:

QKeySequence::QKeySequence(QString const&, QKeySequence::SequenceFormat)

lmoureaux avatar Mar 29 '22 04:03 lmoureaux

I can suppress some qt functions, but I think it is possible that we find flaws in our code where the call is via a Qt container, so I suggest we leave it as it is and ignore the spurious ones.

I'll maybe run another one with a higher variable-tracking-size. Might take a while, but probably gives enough info.

psampathkumar avatar Mar 30 '22 23:03 psampathkumar