Repository: godotengine/godot (language: cpp)

Issue:
[4.4dev7] Crash occurs shortly after clearing ItemList which contains item metadata
### Tested versions

- v4.4.dev4.mono and before this issue did not occur
- Occurs in v4.4.dev7.mono.official [46c8f8c5c] 

### System information

Godot v4.4.dev7.mono - Pop!_OS 22.04 LTS on X11 - X11 display driver, Multi-window, 2 monitors - Vulkan (Forward+) - dedicated AMD Radeon RX 7900 XT (RADV NAVI31) - AMD Ryzen 7 5800X3D 8-Core Processor (16 threads)

### Issue description

A fatal error occurs shortly after I clear a ItemList that contains metadata (the data being a custom class that inherits Node), though not immediately on the clear. I've attempted reproducing in a test project with no luck. If I move this clear to a different point in the script it no longer occurs. 

```
Godot Engine v4.4.dev7.mono.official.46c8f8c5c - https://godotengine.org
Vulkan 1.3.274 - Forward+ - Using Device #0: AMD - AMD Radeon RX 7900 XT (RADV NAVI31)
ERROR: Condition "status < 0" is true. Returning: ERR_CANT_OPEN
   at: init_output_device (drivers/alsa/audio_driver_alsa.cpp:90)
WARNING: All audio drivers failed, falling back to the dummy driver.
   at: initialize (servers/audio_server.cpp:247)

WARNING: Godot 3.x SpatialMaterial remapped parameter not found: flags_world_triplanar
   at: _set (scene/resources/material.cpp:3650)
WARNING: Camera2D overridden to physics process mode due to use of physics interpolation.
   at: _update_process_callback (scene/2d/camera_2d.cpp:92)
WARNING: Nodes with non-equal opposite anchors will have their size overridden after _ready(). 
If you want to set size, change the anchors or consider using set_deferred().
   at: _set_size (scene/gui/control.cpp:1443)
WARNING: Nodes with non-equal opposite anchors will have their size overridden after _ready(). 
If you want to set size, change the anchors or consider using set_deferred().
   at: _set_size (scene/gui/control.cpp:1443)
ERROR: FATAL: Index p_index = 0 is out of bounds (size() = 0).
   at: get (./core/templates/cowdata.h:214)

================================================================
handle_crash: Program crashed with signal 4
Engine version: Godot Engine v4.4.dev7.mono.official (46c8f8c5c5874c7c56ea5b1384259de9402d3449)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[1] /usr/lib/dotnet/shared/Microsoft.NETCore.App/8.0.11/libcoreclr.so(+0x5dd281) [0x7abd3a9dd281] (??:0)
[2] /lib/x86_64-linux-gnu/libc.so.6(+0x42520) [0x7abd75a42520] (??:0)
[3] /home/michaelgooch/GameDev/Godot/Godot_Current/Godot_v4.4-dev7_mono_linux.x86_64() [0x2a63c5d] (??:0)
[4] /home/michaelgooch/GameDev/Godot/Godot_Current/Godot_v4.4-dev7_mono_linux.x86_64() [0x28b72ff] (??:0)
[5] /home/michaelgooch/GameDev/Godot/Godot_Current/Godot_v4.4-dev7_mono_linux.x86_64() [0x2910e15] (??:0)
[6] /home/michaelgooch/GameDev/Godot/Godot_Current/Godot_v4.4-dev7_mono_linux.x86_64() [0x2911fb6] (??:0)
[7] /home/michaelgooch/GameDev/Godot/Godot_Current/Godot_v4.4-dev7_mono_linux.x86_64() [0x293debc] (??:0)
[8] /home/michaelgooch/GameDev/Godot/Godot_Current/Godot_v4.4-dev7_mono_linux.x86_64() [0x4dc3cab] (??:0)
[9] /home/michaelgooch/GameDev/Godot/Godot_Current/Godot_v4.4-dev7_mono_linux.x86_64() [0x4ccdbd] (??:0)
[10] /home/michaelgooch/GameDev/Godot/Godot_Current/Godot_v4.4-dev7_mono_linux.x86_64() [0x436da93] (??:0)
[11] /home/michaelgooch/GameDev/Godot/Godot_Current/Godot_v4.4-dev7_mono_linux.x86_64() [0x436f94f] (??:0)
[12] /home/michaelgooch/GameDev/Godot/Godot_Current/Godot_v4.4-dev7_mono_linux.x86_64() [0x4d48f8] (??:0)
[13] /home/michaelgooch/GameDev/Godot/Godot_Current/Godot_v4.4-dev7_mono_linux.x86_64() [0x41d383] (??:0)
[14] /lib/x86_64-linux-gnu/libc.so.6(+0x29d90) [0x7abd75a29d90] (??:0)
[15] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x80) [0x7abd75a29e40] (??:0)
[16] /home/michaelgooch/GameDev/Godot/Godot_Current/Godot_v4.4-dev7_mono_linux.x86_64() [0x446e6a] (??:0)
-- END OF BACKTRACE --
================================================================
``` 
```gdscript
func add_or_remove_selected_card_holder(target : CardHolder) -> void:
	selected_card_holders.append(target)
	card_holder_target_selected.emit(target)
	targeting_action.set_card_holder_target(selected_card_holders)
	_finish_targeting() #Crashes shortly after here. Script is not really doing anything after this. Nothing touches the item list after this


func _finish_targeting() -> void:
	card_holder_list.clear() #If this is commented out/moved, it no longer crashes. 
	finished_targeting.emit(true)
``` 

### Steps to reproduce

I get the crash 100% in my project in this exact set-up, but can't find a way to reproduce it outside this specific context unfortunately. 

### Minimal reproduction project (MRP)

Haven't been able to make a MRP yet.


Discussion / hints:
If you would be willing to share your project with me as-is I'd be happy to have a look at it. I'm `hp` on the Godot contributors chat, or you can reach me via email on `hp@tmm.cx`, you should also feel free to add me to your github repository if that is easy.

Your code will of course be deleted once the bug is fixed, unless you'd let me hang on to it for future (bug) testing.
I have a reliable stack trace now, the problem appears to be that the items Vector gets emptied while the ui tries to access the list vector. 

Still looking for why/how this happens. It's either a compiler over-optimizing something or a threading issue perhaps.


I found the problem:

in `void ItemList::gui_input(const Ref<InputEvent> &p_event)`

we detect what item is clicked by

`int closest = get_item_at_position(mb->get_position(), true);`

But then on line `emit_signal(SNAME("multi_selected"), i, true);`

We emit a signal, and in the project with the error the signal handler for this clears `items`

So we then 

`if (items[i].selectable && select_mode == SELECT_TOGGLE) {`

Which is where the crash happens, at this point the `closest` value is still set to 1, or 2, but items has been cleared.

Fix patch:
diff --git a/scene/gui/item_list.cpp b/scene/gui/item_list.cpp
index e114aa0452af..302bd0b8d630 100644
--- a/scene/gui/item_list.cpp
+++ b/scene/gui/item_list.cpp
@@ -746,7 +746,19 @@ void ItemList::gui_input(const Ref<InputEvent> &p_event) {
 					return;
 				}
 
-				if (items[i].selectable && (!items[i].selected || allow_reselect) && select_mode != SELECT_TOGGLE) {
+				if (select_mode == SELECT_TOGGLE) {
+					if (items[i].selectable) {
+						if (items[i].selected) {
+							deselect(i);
+							current = i;
+							emit_signal(SNAME("multi_selected"), i, false);
+						} else {
+							select(i, false);
+							current = i;
+							emit_signal(SNAME("multi_selected"), i, true);
+						}
+					}
+				} else if (items[i].selectable && (!items[i].selected || allow_reselect)) {
 					select(i, select_mode == SELECT_SINGLE || !mb->is_command_or_control_pressed());
 
 					if (select_mode == SELECT_SINGLE) {
@@ -756,18 +768,6 @@ void ItemList::gui_input(const Ref<InputEvent> &p_event) {
 					}
 				}
 
-				if (items[i].selectable && select_mode == SELECT_TOGGLE) {
-					if (items[i].selected) {
-						deselect(i);
-						current = i;
-						emit_signal(SNAME("multi_selected"), i, false);
-					} else {
-						select(i, false);
-						current = i;
-						emit_signal(SNAME("multi_selected"), i, true);
-					}
-				}
-
 				emit_signal(SNAME("item_clicked"), i, get_local_mouse_position(), mb->get_button_index());
 
 				if (mb->get_button_index() == MouseButton::LEFT && mb->is_double_click()) {


Test patch: