R20-rc1 HeapUseAfterFree when changig Map during Download

[Klaus Halfmann]

Stnerl was hosting and seleted one Mpa, while still downloading the first on he switched to some other Map which made the game crash with ASAN:

InternetGaming: Client update on metaserver.
[Client] Pong!
[Client] Pong!
[Client] Pong!
[Client] Pong!
[Client] SETTING_MAP 'Astoria 2.R' 'maps/Astoria 2.R.wmf'
[Client] Pong!
[Client] Pong!
[Client] Pong!
[Client] SETTING_MAP 'Srawerb Ait' 'maps/Srawerb Ait.wmf'
=================================================================
==3037==ERROR: AddressSanitizer: heap-use-after-free on address 0x6070003aeab8 at pc 0x00010e9d7cca bp 0x7ffee21e1060 sp 0x7ffee21e1058
READ of size 8 at 0x6070003aeab8 thread T0
    #0 0x10e9d7cc9 in std::__1::__vector_base<FilePart, std::__1::allocator<FilePart> >::~__vector_base() vector:456
    #1 0x10e9d7c84 in std::__1::vector<FilePart, std::__1::allocator<FilePart> >::~vector() iterator:1427
    #2 0x10e9d7c64 in std::__1::vector<FilePart, std::__1::allocator<FilePart> >::~vector() iterator:1427
    #3 0x10e9d7c22 in NetTransferFile::~NetTransferFile() network.h:183
...

0x6070003aeab8 is located 56 bytes inside of 80-byte region [0x6070003aea80,0x6070003aead0)
freed by thread T0 here:
    #0 0x115a77192 in wrap__ZdlPv (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x69192)
    #1 0x10e9bfe27 in GameClient::handle_packet(RecvPacket&) gameclient.cc:590
    #2 0x10e9b73c6 in GameClient::handle_network() gameclient.cc:924
    #3 0x10e9b658b in GameClient::think() gameclient.cc:237
    #4 0x10ef02b77 in FullscreenMenuLaunchMPG::think() launch_mpg.cc:366

previously allocated by thread T0 here:
    #0 0x115a76b92 in wrap__Znwm (libclang_rt.asan_osx_dynamic.dylib:x86_64h+0x68b92)
    #1 0x10e9c0aa3 in GameClient::handle_packet(RecvPacket&) gameclient.cc:643
    #2 0x10e9b73c6 in GameClient::handle_network() gameclient.cc:924
    #3 0x10e9b658b in GameClient::think() gameclient.cc:237
    #4 0x10ef02b77 in FullscreenMenuLaunchMPG::think() launch_mpg.cc:366

Complete Logs attached

[Toni Förster (stonerl)]

[Client] SETTING_MAP '' ''
[Client] Pong!
[Client] Pong!
[Client] SETTING_MAP 'After the Wood Gnomes' 'maps/My_Maps/After the Wood Gnomes.wmf'
[Client] Pong!
[Client] Pong!
[Client] SETTING_MAP 'De Grote Mandrenke' 'maps/My_Maps/fri01.wmf'
widelands(74228,0x11972f5c0) malloc: *** error for object 0x1d00000009: pointer being freed was not allocated
widelands(74228,0x11972f5c0) malloc: *** set a breakpoint in malloc_error_break to debug
Abort trap: 6

[Klaus Halfmann (klaus-halfmann)]

Happens only rarely so perhaps not critical?

[Klaus Halfmann (klaus-halfmann)]

Some analysis:

* GameClient::handle_packet deserves a refactoring that function ist just to big -> R21
* NetTransferFile is only a struct Ctor / DTor are created implicitly
  -> Will create a local branch for debugging based on build20
* GameClient is owner of NetTransferFile* file_, does not set it to null on delete?
  -> lets try that one ...

[GunChleoc (gunchleoc)]

Fixed in build20