Skip to content

Commit

Permalink
Mutex guard tile_map_layer dirty.cell_list.
Browse files Browse the repository at this point in the history
TilesEditorUtils calls tile_map->set_pattern from a thread (to refresh
tile pattern preview images). set_pattern sets each of the cells into the
temporary map, marking them dirty via set_cell.

This can race against the tree invoking the TileMapLayer's
internal_update, which proceses dirty.cell_list and eventually clears
it. If a cell's shared dirty entry ends up losing the race, clear can
get stuck in an infinite loop where remove refuses to change it because
the entry doesn't think it's a member of the list, but the list head
still points to it. This spams

ERROR: Condition "p_elem->_root != this" is true.
   at: remove (./core/templates/self_list.h:80)

And eventually leads to a crash when the error message buffer fills.

Guarding the list mutations avoids the race that sets up for this
failure, fixing #86226.
  • Loading branch information
baptr committed Jan 22, 2024
1 parent 0bcc0e9 commit 873ca32
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 1 deletion.
5 changes: 4 additions & 1 deletion scene/2d/tile_map_layer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2111,6 +2111,8 @@ void TileMapLayer::notify_tile_map_change(DirtyFlags p_what) {
}

void TileMapLayer::internal_update() {
MutexLock lock(dirty.mutex);

// Find TileData that need a runtime modification.
// This may add cells to the dirty list is a runtime modification has been notified.
_build_runtime_update_tile_data();
Expand Down Expand Up @@ -2187,6 +2189,7 @@ void TileMapLayer::set_cell(const Vector2i &p_coords, int p_source_id, const Vec
c.alternative_tile = alternative_tile;

// Make the given cell dirty.
MutexLock lock(dirty.mutex);
if (!E->value.dirty_list_element.in_list()) {
dirty.cell_list.add(&(E->value.dirty_list_element));
}
Expand Down Expand Up @@ -2939,4 +2942,4 @@ TerrainConstraint::TerrainConstraint(Ref<TileSet> p_tile_set, const Vector2i &p_
}
}
terrain = p_terrain;
}
}
2 changes: 2 additions & 0 deletions scene/2d/tile_map_layer.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
#ifndef TILE_MAP_LAYER_H
#define TILE_MAP_LAYER_H

#include "core/os/mutex.h"
#include "scene/2d/tile_map.h"
#include "scene/resources/tile_set.h"

Expand Down Expand Up @@ -262,6 +263,7 @@ class TileMapLayer : public Node2D {

// Dirty flag. Allows knowing what was modified since the last update.
struct {
Mutex mutex;
bool flags[DIRTY_FLAGS_MAX] = { false };
SelfList<CellData>::List cell_list;
} dirty;
Expand Down

0 comments on commit 873ca32

Please sign in to comment.