TileSet-related crash when closing the editor

[KoBeWi]

Godot version

4.2 3fa8fad

System information

Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce GTX 1060 (NVIDIA; 30.0.15.1403) - Intel(R) Core(TM) i7-7700HQ CPU @ 2.80GHz (8 Threads)

Issue description

CrashHandlerException: Program crashed
Engine version: Godot Engine v4.2.dev.custom_build (3fa8fad26b97a8af20e7996b7e17d8f23fc04b89)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[0] SafeRefCount::_check_unref_sanity (C:\godot_source\core\templates\safe_refcount.h:187)
[1] SafeRefCount::unrefval (C:\godot_source\core\templates\safe_refcount.h:214)
[2] RefCounted::unreference (C:\godot_source\core\object\ref_counted.cpp:77)
[3] Ref<TileSet>::unref (C:\godot_source\core\object\ref_counted.h:209)
[4] Ref<TileSet>::ref (C:\godot_source\core\object\ref_counted.h:67)
[5] Ref<TileSet>::operator= (C:\godot_source\core\object\ref_counted.h:118)
[6] TileSetAtlasSourceEditor::_tile_set_changed (C:\godot_source\editor\plugins\tiles\tile_set_atlas_source_editor.cpp:2085)
[7] call_with_variant_args_helper<TileSetAtlasSourceEditor> (C:\godot_source\core\variant\binder_common.h:308)
[8] call_with_variant_args<TileSetAtlasSourceEditor> (C:\godot_source\core\variant\binder_common.h:418)
[9] CallableCustomMethodPointer<TileSetAtlasSourceEditor>::call (C:\godot_source\core\object\callable_method_pointer.h:105)
[10] Callable::callp (C:\godot_source\core\variant\callable.cpp:51)
[11] Object::emit_signalp (C:\godot_source\core\object\object.cpp:1073)
[12] Object::emit_signal<> (C:\godot_source\core\object\object.h:891)
[13] Resource::emit_changed (C:\godot_source\core\io\resource.cpp:45)
[14] TileSet::remove_source (C:\godot_source\scene\resources\tile_set.cpp:510)
[15] TileSet::~TileSet (C:\godot_source\scene\resources\tile_set.cpp:3602)
[16] TileSet::`scalar deleting destructor'
[17] memdelete<TileSet> (C:\godot_source\core\os\memory.h:112)
[18] Ref<TileSet>::unref (C:\godot_source\core\object\ref_counted.h:212)
[19] Ref<TileSet>::~Ref<TileSet> (C:\godot_source\core\object\ref_counted.h:222)
[20] TileSetAtlasSourceEditor::~TileSetAtlasSourceEditor (C:\godot_source\editor\plugins\tiles\tile_set_atlas_source_editor.cpp:2657)
[21] TileSetAtlasSourceEditor::`scalar deleting destructor'
[22] memdelete<Node> (C:\godot_source\core\os\memory.h:112)
[23] Node::_notification (C:\godot_source\scene\main\node.cpp:212)
[24] Node::_notificationv (C:\godot_source\scene\main\node.h:49)
[25] CanvasItem::_notificationv (C:\godot_source\scene\main\canvas_item.h:45)
[26] Control::_notificationv (C:\godot_source\scene\gui\control.h:47)
[27] Container::_notificationv (C:\godot_source\scene\gui\container.h:37)
[28] BoxContainer::_notificationv (C:\godot_source\scene\gui\box_container.h:37)
[29] VBoxContainer::_notificationv (C:\godot_source\scene\gui\box_container.h:92)
[30] Object::notification (C:\godot_source\core\object\object.cpp:800)
[31] Object::_predelete (C:\godot_source\core\object\object.cpp:198)
[32] predelete_handler (C:\godot_source\core\object\object.cpp:1893)
[33] memdelete<Node> (C:\godot_source\core\os\memory.h:105)
[34] Node::_notification (C:\godot_source\scene\main\node.cpp:212)
[35] Node::_notificationv (C:\godot_source\scene\main\node.h:49)
[36] CanvasItem::_notificationv (C:\godot_source\scene\main\canvas_item.h:45)
[37] Control::_notificationv (C:\godot_source\scene\gui\control.h:47)
[38] Container::_notificationv (C:\godot_source\scene\gui\container.h:37)
[39] SplitContainer::_notificationv (C:\godot_source\scene\gui\split_container.h:54)
[40] HSplitContainer::_notificationv (C:\godot_source\scene\gui\split_container.h:124)
[41] Object::notification (C:\godot_source\core\object\object.cpp:800)
[42] Object::_predelete (C:\godot_source\core\object\object.cpp:198)
[43] predelete_handler (C:\godot_source\core\object\object.cpp:1893)
[44] memdelete<Node> (C:\godot_source\core\os\memory.h:105)
[45] Node::_notification (C:\godot_source\scene\main\node.cpp:212)
[46] Node::_notificationv (C:\godot_source\scene\main\node.h:49)
[47] CanvasItem::_notificationv (C:\godot_source\scene\main\canvas_item.h:45)
[48] Control::_notificationv (C:\godot_source\scene\gui\control.h:47)
[49] Container::_notificationv (C:\godot_source\scene\gui\container.h:37)
[50] BoxContainer::_notificationv (C:\godot_source\scene\gui\box_container.h:37)
[51] VBoxContainer::_notificationv (C:\godot_source\scene\gui\box_container.h:92)
[52] Object::notification (C:\godot_source\core\object\object.cpp:800)
[53] Object::_predelete (C:\godot_source\core\object\object.cpp:198)
[54] predelete_handler (C:\godot_source\core\object\object.cpp:1893)
[55] memdelete<Node> (C:\godot_source\core\os\memory.h:105)
[56] Node::_notification (C:\godot_source\scene\main\node.cpp:212)
[57] Node::_notificationv (C:\godot_source\scene\main\node.h:49)
[58] CanvasItem::_notificationv (C:\godot_source\scene\main\canvas_item.h:45)
[59] Control::_notificationv (C:\godot_source\scene\gui\control.h:47)
[60] TileSetEditor::_notificationv (C:\godot_source\editor\plugins\tiles\tile_set_editor.h:46)
[61] Object::notification (C:\godot_source\core\object\object.cpp:800)
[62] Object::_predelete (C:\godot_source\core\object\object.cpp:198)
[63] predelete_handler (C:\godot_source\core\object\object.cpp:1893)
[64] memdelete<Node> (C:\godot_source\core\os\memory.h:105)
[65] Node::_notification (C:\godot_source\scene\main\node.cpp:212)
[66] Node::_notificationv (C:\godot_source\scene\main\node.h:49)
[67] CanvasItem::_notificationv (C:\godot_source\scene\main\canvas_item.h:45)
[68] Control::_notificationv (C:\godot_source\scene\gui\control.h:47)
[69] Container::_notificationv (C:\godot_source\scene\gui\container.h:37)
[70] BoxContainer::_notificationv (C:\godot_source\scene\gui\box_container.h:37)
[71] VBoxContainer::_notificationv (C:\godot_source\scene\gui\box_container.h:92)
[72] Object::notification (C:\godot_source\core\object\object.cpp:800)
[73] Object::_predelete (C:\godot_source\core\object\object.cpp:198)
[74] predelete_handler (C:\godot_source\core\object\object.cpp:1893)
[75] memdelete<Node> (C:\godot_source\core\os\memory.h:105)
[76] Node::_notification (C:\godot_source\scene\main\node.cpp:212)
[77] Node::_notificationv (C:\godot_source\scene\main\node.h:49)
[78] CanvasItem::_notificationv (C:\godot_source\scene\main\canvas_item.h:45)
[79] Control::_notificationv (C:\godot_source\scene\gui\control.h:47)
[80] Container::_notificationv (C:\godot_source\scene\gui\container.h:37)
[81] PanelContainer::_notificationv (C:\godot_source\scene\gui\panel_container.h:37)
[82] Object::notification (C:\godot_source\core\object\object.cpp:800)
[83] Object::_predelete (C:\godot_source\core\object\object.cpp:198)
[84] predelete_handler (C:\godot_source\core\object\object.cpp:1893)
[85] memdelete<Node> (C:\godot_source\core\os\memory.h:105)
[86] Node::_notification (C:\godot_source\scene\main\node.cpp:212)
[87] Node::_notificationv (C:\godot_source\scene\main\node.h:49)
[88] CanvasItem::_notificationv (C:\godot_source\scene\main\canvas_item.h:45)
[89] Control::_notificationv (C:\godot_source\scene\gui\control.h:47)
[90] Container::_notificationv (C:\godot_source\scene\gui\container.h:37)
[91] SplitContainer::_notificationv (C:\godot_source\scene\gui\split_container.h:54)
[92] VSplitContainer::_notificationv (C:\godot_source\scene\gui\split_container.h:132)
[93] Object::notification (C:\godot_source\core\object\object.cpp:800)
[94] Object::_predelete (C:\godot_source\core\object\object.cpp:198)
[95] predelete_handler (C:\godot_source\core\object\object.cpp:1893)
[96] memdelete<Node> (C:\godot_source\core\os\memory.h:105)
[97] Node::_notification (C:\godot_source\scene\main\node.cpp:212)
[98] Node::_notificationv (C:\godot_source\scene\main\node.h:49)
[99] CanvasItem::_notificationv (C:\godot_source\scene\main\canvas_item.h:45)
[100] Control::_notificationv (C:\godot_source\scene\gui\control.h:47)
[101] Container::_notificationv (C:\godot_source\scene\gui\container.h:37)
[102] BoxContainer::_notificationv (C:\godot_source\scene\gui\box_container.h:37)
[103] VBoxContainer::_notificationv (C:\godot_source\scene\gui\box_container.h:92)
[104] Object::notification (C:\godot_source\core\object\object.cpp:800)
[105] Object::_predelete (C:\godot_source\core\object\object.cpp:198)
[106] predelete_handler (C:\godot_source\core\object\object.cpp:1893)
[107] memdelete<Node> (C:\godot_source\core\os\memory.h:105)
[108] Node::_notification (C:\godot_source\scene\main\node.cpp:212)
[109] Node::_notificationv (C:\godot_source\scene\main\node.h:49)
[110] CanvasItem::_notificationv (C:\godot_source\scene\main\canvas_item.h:45)
[111] Control::_notificationv (C:\godot_source\scene\gui\control.h:47)
[112] Container::_notificationv (C:\godot_source\scene\gui\container.h:37)
[113] SplitContainer::_notificationv (C:\godot_source\scene\gui\split_container.h:54)
[114] HSplitContainer::_notificationv (C:\godot_source\scene\gui\split_container.h:124)
[115] Object::notification (C:\godot_source\core\object\object.cpp:800)
[116] Object::_predelete (C:\godot_source\core\object\object.cpp:198)
[117] predelete_handler (C:\godot_source\core\object\object.cpp:1893)
[118] memdelete<Node> (C:\godot_source\core\os\memory.h:105)
[119] Node::_notification (C:\godot_source\scene\main\node.cpp:212)
[120] Node::_notificationv (C:\godot_source\scene\main\node.h:49)
[121] CanvasItem::_notificationv (C:\godot_source\scene\main\canvas_item.h:45)
[122] Control::_notificationv (C:\godot_source\scene\gui\control.h:47)
[123] Container::_notificationv (C:\godot_source\scene\gui\container.h:37)
[124] SplitContainer::_notificationv (C:\godot_source\scene\gui\split_container.h:54)
[125] HSplitContainer::_notificationv (C:\godot_source\scene\gui\split_container.h:124)
[126] Object::notification (C:\godot_source\core\object\object.cpp:800)
[127] Object::_predelete (C:\godot_source\core\object\object.cpp:198)
[128] predelete_handler (C:\godot_source\core\object\object.cpp:1893)
[129] memdelete<Node> (C:\godot_source\core\os\memory.h:105)
[130] Node::_notification (C:\godot_source\scene\main\node.cpp:212)
[131] Node::_notificationv (C:\godot_source\scene\main\node.h:49)
[132] CanvasItem::_notificationv (C:\godot_source\scene\main\canvas_item.h:45)
[133] Control::_notificationv (C:\godot_source\scene\gui\control.h:47)
[134] Container::_notificationv (C:\godot_source\scene\gui\container.h:37)
[135] SplitContainer::_notificationv (C:\godot_source\scene\gui\split_container.h:54)
[136] HSplitContainer::_notificationv (C:\godot_source\scene\gui\split_container.h:124)
[137] Object::notification (C:\godot_source\core\object\object.cpp:800)
[138] Object::_predelete (C:\godot_source\core\object\object.cpp:198)
[139] predelete_handler (C:\godot_source\core\object\object.cpp:1893)
[140] memdelete<Node> (C:\godot_source\core\os\memory.h:105)
[141] Node::_notification (C:\godot_source\scene\main\node.cpp:212)
[142] Node::_notificationv (C:\godot_source\scene\main\node.h:49)
[143] CanvasItem::_notificationv (C:\godot_source\scene\main\canvas_item.h:45)
[144] Control::_notificationv (C:\godot_source\scene\gui\control.h:47)
[145] Container::_notificationv (C:\godot_source\scene\gui\container.h:37)
[146] BoxContainer::_notificationv (C:\godot_source\scene\gui\box_container.h:37)
[147] VBoxContainer::_notificationv (C:\godot_source\scene\gui\box_container.h:92)
[148] Object::notification (C:\godot_source\core\object\object.cpp:800)
[149] Object::_predelete (C:\godot_source\core\object\object.cpp:198)
[150] predelete_handler (C:\godot_source\core\object\object.cpp:1893)
[151] memdelete<Node> (C:\godot_source\core\os\memory.h:105)
[152] Node::_notification (C:\godot_source\scene\main\node.cpp:212)
[153] Node::_notificationv (C:\godot_source\scene\main\node.h:49)
[154] CanvasItem::_notificationv (C:\godot_source\scene\main\canvas_item.h:45)
[155] Control::_notificationv (C:\godot_source\scene\gui\control.h:47)
[156] Panel::_notificationv (C:\godot_source\scene\gui\panel.h:37)
[157] Object::notification (C:\godot_source\core\object\object.cpp:800)
[158] Object::_predelete (C:\godot_source\core\object\object.cpp:198)
[159] predelete_handler (C:\godot_source\core\object\object.cpp:1893)
[160] memdelete<Node> (C:\godot_source\core\os\memory.h:105)
[161] Node::_notification (C:\godot_source\scene\main\node.cpp:212)
[162] Node::_notificationv (C:\godot_source\scene\main\node.h:49)
[163] CanvasItem::_notificationv (C:\godot_source\scene\main\canvas_item.h:45)
[164] Control::_notificationv (C:\godot_source\scene\gui\control.h:47)
[165] Object::notification (C:\godot_source\core\object\object.cpp:800)
[166] Object::_predelete (C:\godot_source\core\object\object.cpp:198)
[167] predelete_handler (C:\godot_source\core\object\object.cpp:1893)
[168] memdelete<Node> (C:\godot_source\core\os\memory.h:105)
[169] Node::_notification (C:\godot_source\scene\main\node.cpp:212)
[170] Node::_notificationv (C:\godot_source\scene\main\node.h:49)
[171] EditorNode::_notificationv (C:\godot_source\editor\editor_node.h:118)
[172] Object::notification (C:\godot_source\core\object\object.cpp:800)
[173] Object::_predelete (C:\godot_source\core\object\object.cpp:198)
[174] predelete_handler (C:\godot_source\core\object\object.cpp:1893)
[175] memdelete<Node> (C:\godot_source\core\os\memory.h:105)
[176] Node::_notification (C:\godot_source\scene\main\node.cpp:212)
[177] Node::_notificationv (C:\godot_source\scene\main\node.h:49)
[178] Viewport::_notificationv (C:\godot_source\scene\main\viewport.h:93)
[179] Window::_notificationv (C:\godot_source\scene\main\window.h:44)
[180] Object::notification (C:\godot_source\core\object\object.cpp:800)
[181] Object::_predelete (C:\godot_source\core\object\object.cpp:198)
[182] predelete_handler (C:\godot_source\core\object\object.cpp:1893)
[183] memdelete<Window> (C:\godot_source\core\os\memory.h:105)
[184] SceneTree::finalize (C:\godot_source\scene\main\scene_tree.cpp:629)
[185] OS_Windows::run (C:\godot_source\platform\windows\os_windows.cpp:1485)
[186] widechar_main (C:\godot_source\platform\windows\godot_windows.cpp:182)
[187] _main (C:\godot_source\platform\windows\godot_windows.cpp:204)
[188] main (C:\godot_source\platform\windows\godot_windows.cpp:218)
[189] WinMain (C:\godot_source\platform\windows\godot_windows.cpp:232)
[190] __scrt_common_main_seh (D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
[191] <couldn't map PC to fn name>
-- END OF BACKTRACE --

Steps to reproduce

1. Open the MRP
2. Open the scene
3. Click the TileMap node
4. Unfold the TileSet in the inspector
5. Click empty space in scene tree to deselect TileMap
6. Exit the editor

Minimal reproduction project

tilemap.zip

[lawnjelly]

Maybe it's happening because _tile_set_changed() is being called one more time after the tile_set->get_source_count() == 0, so the disconnect is being attempted twice.

It should be set elsewhere that the tileset has been disconnected, so it isn't done twice, maybe. Or just check if tile_set == Ref<TileSet>() before trying the disconnect?

Interesting that it seems to be happening on the second line rather than the first: 🤔

tile_set->disconnect_changed(callable_mp(this, &TileSetAtlasSourceEditor::_tile_set_changed));
tile_set = Ref<TileSet>();

An MRP with a tilemap and tileset would be handy to reproduce this for those of us who are not used to making tilesets. 😁

[KoBeWi]

Can no longer reproduce on master, #80462 might've fixed it accidentally.

Here's MRP if someone wants to test anyway:
tilemap.zip

[KoBeWi]

Ok it just happened again. I updated the reproduction steps.

[lawnjelly]

I couldn't get it to reproduce by closing the editor, but I could by closing just the specific scene tab (maybe it was just random chance).
Stack trace is different though, segmentation fault and I'm not sure exactly what causes the crash, perhaps tilemap pointer accessed after deletion.

1   TileMapEditorPlugin::edit                                                                                                                                       tiles_editor_plugin.cpp   351  0x55555d13b2e4 
2   EditorNode::_plugin_over_edit                                                                                                                                   editor_node.cpp           890  0x55555bf70ce7 
3   EditorNode::hide_unused_editors                                                                                                                                 editor_node.cpp           2242 0x55555bf88b6c 
4   SceneTreeDock::_push_item                                                                                                                                       scene_tree_dock.cpp       1488 0x55555c3386e2 
5   SceneTreeDock::_selection_changed                                                                                                                               scene_tree_dock.cpp       2276 0x55555c3582a1 
6   call_with_variant_args_helper<SceneTreeDock>(SceneTreeDock *, void (SceneTreeDock:: *)(), Variant const * *, Callable::CallError&, IndexSequence<>)             binder_common.h           303  0x55555c42af09 
7   call_with_variant_args<SceneTreeDock>                                                                                                                           binder_common.h           417  0x55555c42ae79 
8   CallableCustomMethodPointer<SceneTreeDock>::call                                                                                                                callable_method_pointer.h 104  0x55555c42ad64 
9   Callable::callp                                                                                                                                                 callable.cpp              50   0x55555f921a7a 
10  Object::emit_signalp                                                                                                                                            object.cpp                1072 0x55555fcff623 
11  Object::emit_signal<>(StringName const&)                                                                                                                        object.h                  891  0x55555b28b203 
12  EditorSelection::_emit_change                                                                                                                                   editor_data.cpp           1292 0x55555bd07649 
13  call_with_variant_args_helper<__UnexistingClass>(__UnexistingClass *, void (__UnexistingClass:: *)(), Variant const * *, Callable::CallError&, IndexSequence<>) binder_common.h           303  0x55555a859f19 
14  call_with_variant_args_dv<__UnexistingClass>                                                                                                                    binder_common.h           450  0x55555a859e7c 
15  MethodBindT<>::call(Object *, Variant const * *, int, Callable::CallError&) const                                                                               method_bind.h             331  0x55555a859c46 
16  Object::callp                                                                                                                                                   object.cpp                741  0x55555fcfcdcf 
17  Callable::callp                                                                                                                                                 callable.cpp              62   0x55555f921e77 
18  CallQueue::_call_function                                                                                                                                       message_queue.cpp         219  0x55555fcf7640 
19  CallQueue::flush                                                                                                                                                message_queue.cpp         324  0x55555fcf7eca 
20  SceneTree::physics_process                                                                                                                                      scene_tree.cpp            471  0x55555d2b1935 
... <More>                                                                                                                                                                                                        

It's crashing here:

void TileMapEditorPlugin::edit(Object *p_object) {
	if (tile_map) {
		tile_map->disconnect("changed", callable_mp(this, &TileMapEditorPlugin::_tile_map_changed));
	}

Where p_object is NULL, so it looks like it's trying to wipe the old tilemap from the plugin, but it is still accessing the old tilemap. So maybe this could be order of deletion issue, or that this function is assuming that the old tilemap is still around? 🤔

Whether this is the same bug or just a related bug I don't know.

UPDATE:
Yes, I can confirm that at this point tile_map has been deleted already.
There's no need to call this on a deleted tilemap as the destructor already does this:

TileMap::~TileMap() {
	if (tile_set.is_valid()) {
		tile_set->disconnect_changed(callable_mp(this, &TileMap::_tile_set_changed));
	}

So this just needs to presumably be called on tilemaps that have not been deleted, but either not here, or by checking their ObjectID is still valid or something like that (or a signal to let tilemap editor know tilemap is being destroyed). 🤔

I've added example PR for fixing this, but feel free to do your own just in case I am away this weekend.

[lawnjelly]

Am suspecting that the original bug mentioned by @KoBeWi is similar. I can't reproduce it (maybe a timing issue?) but if tile_set has been deleted at this point you could also check tile_set ObjectID in a similar manner to my PR (or other methods to remove the reference).

_tile_set_changed() doesn't appear to be called on exit on mine when using the MRP and following the steps, so not surprising no crash. So perhaps there is something in the reproduction steps which is missing.

[lawnjelly]

Finally having a look at the original bug.

It looks like the problem could be that TileSetAtlasSourceEditor::_tile_set_changed() is being called after TileSetAtlasSourceEditor has been destroyed.

The tileset has previously been released (to RefCount 0) but curiously has not been destroyed, maybe that's an order of operations thing. It is likely that fixing the above bug will fix this.

TileSetAtlasSourceEditor::_tile_set_changed() is being called via a signal on the TileSet. I'm not quite sure how this is handled in master (not being familiar), whether TileSetAtlasSourceEditor is meant to remove its slot from TileSet during destruction, or whether the signal is meant to check objects are valid before calling them. Suspect it is the former.

Ok I have a fix based on the above, will PR. Can't guarantee there aren't more bugs in this stuff, but at least it fixes this crash.