Crash in defaultai

[SirVer]

I let all AIs play for some minutes on a big map, and got this crash in check_enemy_sites(). Unfortunately this was a release build, so no more information.

Thread 0 Crashed:: Dispatch queue: com.apple.main-thread
0 widelands 0x000000010678ac63 DefaultAI::check_enemy_sites(unsigned int) + 6771
1 widelands 0x000000010677d626 DefaultAI::think() + 1766
2 widelands 0x000000010661a07a NetHost::think() + 602
3 widelands 0x0000000106557187 Widelands::Game::think() + 23
4 widelands 0x00000001066f7e8b InteractiveBase::think() + 411
5 widelands 0x0000000106651a08 UI::Panel::do_think() + 24
6 widelands 0x00000001066516dc UI::Panel::do_run() + 396

[TiborB (tiborb95)]

Similar report bug-1330070, only there it crashed in check_productionsites(int), but again it was apple.

When debugging with gdb it would show couple of lines above the line '0' and list also the lines in the code...

Do you have line "Crashed Thread:" there?

It would be preferable to run it as debug build, those asserts are there for reason.

This part of AI code was not touched for long time. I would suggest the problem is elsewhere, perhaps with lates UI changes, f.e. I reported this bug-1530723 I got an assert invoked crash but without assert it might behave who knows how

[SirVer (sirver)]

This is a backtrace that I got in debug build:

(gdb) bt
#0 0x0000000100027a3a in ?? ()
#1 0x00007fff5fbf4f20 in ?? ()
#2 0x0000000100922f6e in operator++ (this=0x12f012360) at /usr/local/Cellar/llvm36/3.6.0/lib/llvm-3.6/include/c++/v1/__tree:656
#3 operator++ (this=0x0) at /usr/local/Cellar/llvm36/3.6.0/lib/llvm-3.6/include/c++/v1/map:680
#4 DefaultAI::check_enemy_sites (this=0x1035da000, gametime=0) at /Users/sirver/Desktop/Programming/cpp/widelands/bzr_repo/src/ai/defaultai.cc:5357

I see the problem there too:

for (std::map<uint32_t, EnemySiteObserver>::iterator site = enemy_sites.begin();
          site != enemy_sites.end();
         ++site) {
/* a lot of code */

      if (site_to_be_removed < std::numeric_limits<uint32_t>::max()) {
          enemy_sites.erase(site_to_be_removed); // <-- This line modifies the container and possibly invalidates 'site'.
          continue;
       }
}

std::map::erase() invalidates all iterators pointing to the element being removed. That seem to can happen in this case. One possible solution is to use the new c++11 erase, which returns an iterator pointing to the next element after the one that is removed: site = enemy_site.erase(), but I am not sure if that is compatible with the logic you want here, Tibor.

Could bug 1330070 be a similar issue?

[TiborB (tiborb95)]

You got it. Strangely it never causes problems under linux

The fix would be just collecting (into vector f.e.) sites (uint32_t) to be erased and do it afterwards in a safe way.

as for bug 1330070 - there is no erase in check_productionsites() so this is not the same issue

[TiborB (tiborb95)]

Should be fixed in 7691

[GunChleoc (gunchleoc)]

Fixed in build19-rc1.