Skip to content

Commit

Permalink
Bugfix: Prevent infinite loop when spawning monsters
Browse files Browse the repository at this point in the history
* Prevents game occasionally seemingly hanging when moving to new submaps

The reason for the previous problem was an infinite loop caused by:
1. `map::spawn_monsters_submap` for-loops the list of `current_submap->spawns`
2. for every spawned monster, it calls `monster::on_load`
3. `monster::on_load` calls `monster::try_reproduce`, which in turn calls
   `map::add_spawn`
4. So a new spawn is added, thus invalidating the iterator used in step 1
5. Undefined behavior caused by using invaliated iterators.

On my compiler (gcc 13.2.0), the above problem had the following effect:
* The reference `spawn_point &i` pointed to something totally different, so
  that in particular, `i.count` had garbage values
* Instead of `i.count` being reasonable values such as `3` or `1`, the above
  undefined behavior made it have values such as `925969776` or `-632214304`
* `i.count` is the upper bound for the inner for-loop in
  `map::spawn_monsters_submap`, so depending on the garbage value, it might
  seem like an infinite loop.

Stacktrace of app when frozen and problem happened:
```
 #0  0x000055a1eaf36dcb in creature_tracker::find(coords::coord_point<tripoint, (coords::origin)1, (coords::scale)0> const&) const ()
 #1  0x000055a1eaf39253 in Creature* creature_tracker::creature_at<Creature>(coords::coord_point<tripoint, (coords::origin)1, (coords::scale)0> const&, bool) ()
 #2  0x000055a1eaf393b5 in Creature* creature_tracker::creature_at<Creature>(tripoint const&, bool) ()
 #3  0x000055a1eb357a53 in map::spawn_monsters_submap(tripoint const&, bool, bool)::{lambda(tripoint const&)#1}::operator()(tripoint const&) const ()
 #4  0x000055a1eb3a976e in random_point(tripoint_range<tripoint> const&, std::function<bool (tripoint const&)> const&) ()
 #5  0x000055a1eb37dd2d in map::spawn_monsters_submap(tripoint const&, bool, bool) ()
 #6  0x000055a1eb37de77 in map::spawn_monsters(bool, bool) ()
 #7  0x000055a1eb093981 in game::update_map(int&, int&, bool) ()
 #8  0x000055a1eb094451 in game::update_map(Character&, bool) ()
 #9  0x000055a1eb09530f in game::place_player(tripoint const&, bool) ()
 #10 0x000055a1eb0b3a0e in game::walk_move(tripoint const&, bool, bool) ()
 #11 0x000055a1ead27490 in avatar_action::move(avatar&, map&, tripoint const&) ()
 #12 0x000055a1eb0f338c in game::do_regular_action(action_id&, avatar&, std::optional<tripoint> const&) ()
 #13 0x000055a1eb0f6e63 in game::handle_action() ()
 #14 0x000055a1eafbd9ea in do_turn() ()
 #15 0x000055a1eaa5ec13 in main ()
```

This commit instead changes the loop in step 1 above so that it explicitly
*not* uses iterators, but instead old-fashioned indexed loop. The intention
with the change is to allow other parts of the code to add items to the vector
`current_submap->spawns` while we are iterating it here. If new items are
added, they will be handled in later steps of the loop.
  • Loading branch information
inogenous committed Nov 19, 2023
1 parent eca466b commit d08bff6
Showing 1 changed file with 7 additions and 1 deletion.
8 changes: 7 additions & 1 deletion src/map.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -8720,7 +8720,13 @@ void map::spawn_monsters_submap( const tripoint &gp, bool ignore_sight, bool spa
const tripoint gp_ms = sm_to_ms_copy( gp );

creature_tracker &creatures = get_creature_tracker();
for( spawn_point &i : current_submap->spawns ) {

// The list of spawns on the submap might be updated while we are iterating it.
// For example, `monster::on_load` -> `monster::try_reproduce` calls `map::add_spawn`.
// Therefore, this intentionally uses old-school indexed for-loop with re-check against `.size()` each step.
// NOLINTNEXTLINE(modernize-loop-convert)
for( size_t sp_i = 0; sp_i < current_submap->spawns.size(); ++sp_i ) {
const spawn_point i = current_submap->spawns[sp_i]; // intentional copy
const tripoint center = gp_ms + i.pos;
const tripoint_range<tripoint> points = points_in_radius( center, 3 );

Expand Down

0 comments on commit d08bff6

Please sign in to comment.