Repository: godotengine/godot (language: cpp)

Issue:
Editor crashes when callback for `RD::texture_get_data_async` is an empty method


### Tested versions

- Reproducable in 4.4-beta2, 4.4-beta1, 4.4-dev7

(All versions with [#100110](https://github.com/godotengine/godot/pull/100110))

### System information

Godot v4.4.beta2 - Windows 11 (build 22631) - Multi-window, 2 monitors - Vulkan (Forward+) - dedicated NVIDIA GeForce RTX 3060 Ti (NVIDIA; 32.0.15.6094) - 12th Gen Intel(R) Core(TM) i5-12400F (12 threads)

### Issue description

When calling `RD::texture_get_data_async` from a tool script on every process and passing a callback to an empty method the Editor crashes after about 5 seconds.

```
ERROR: FATAL: Index p_index = 8278 is out of bounds (count = 82).
   at: operator[] (./core/templates/local_vector.h:178)

================================================================
CrashHandlerException: Program crashed with signal 4
Engine version: Godot Engine v4.4.beta2.official (a013481b0911e59cc3f3dea7ebb732450c3e1460)
Dumping the backtrace. Please include this when reporting the bug to the project developer.

(no backtrace.. same with --verbose)
```

Running the game does not trigger the crash. It only seems to be happening in the editor.

Having something happen in the callback also doesn't seem to trigger the crash or perhaps delays it to great extend. I doubt that the empty callback is the root cause. It probably just triggers whatever is wrong with using it in the editor faster. Though, I'm speculating here.

If this wasn't intended to work in the editor, there should be a guard.

### Steps to reproduce

- Uncommennt `@tool` at the top of node.gd
- save
- reopen the scene
- switch to 2D
- wait for about 5s

Also try uncommenting the print statement in the callback. That seems to stop the crash.

### Minimal reproduction project (MRP)

[texture-get-data-async-empty-callback-editor-crash.zip](https://github.com/user-attachments/files/18656439/texture-get-data-async-empty-callback-editor-crash.zip)


Discussion / hints:
Also crashes on my Linux install.


Godot v4.4.beta2 - Debian GNU/Linux 12 (bookworm) 12 on Wayland - X11 display driver, Multi-window, 1 monitor - Vulkan (Forward+) - integrated Intel(R) UHD Graphics 620 (WHL GT2) - Intel(R) Core(TM) i5-8365U CPU @ 1.60GHz (8 threads)

```
ERROR: FATAL: Index p_index = 8469 is out of bounds (count = 171).
   at: operator[] (./core/templates/local_vector.h:178)

================================================================
handle_crash: Program crashed with signal 4
Engine version: Godot Engine v4.4.beta2.official (a013481b0911e59cc3f3dea7ebb732450c3e1460)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[1] /lib/x86_64-linux-gnu/libc.so.6(+0x3c050) [0x7f152fc99050] (??:0)
[2] /home/henry/godot44.x86_64() [0x3d88fb6] (??:0)
[3] /home/henry/godot44.x86_64() [0x3e14b80] (??:0)
[4] /home/henry/godot44.x86_64() [0xb822bf] (??:0)
[5] /home/henry/godot44.x86_64() [0xa087e6] (??:0)
[6] /home/henry/godot44.x86_64() [0x28a33ba] (??:0)
[7] /home/henry/godot44.x86_64() [0x4735d08] (??:0)
[8] /home/henry/godot44.x86_64() [0x28b42ac] (??:0)
[9] /home/henry/godot44.x86_64() [0x29032a3] (??:0)
[10] /home/henry/godot44.x86_64() [0x29038c9] (??:0)
[11] /home/henry/godot44.x86_64() [0x50ea5d] (??:0)
[12] /home/henry/godot44.x86_64() [0x4208c3] (??:0)
[13] /lib/x86_64-linux-gnu/libc.so.6(+0x2724a) [0x7f152fc8424a] (??:0)
[14] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x85) [0x7f152fc84305] (??:0)
[15] /home/henry/godot44.x86_64() [0x44359a] (??:0)
-- END OF BACKTRACE --
================================================================
```
Confirmed in the current `master` branch (eee39f004b42a4948af90cdebedf65c34a4a4970). Backtrace:
```
ERROR: FATAL: Index p_index = 8304 is out of bounds (count = 56).
   at: operator[] (./core/templates/local_vector.h:178)

================================================================
handle_crash: Program crashed with signal 4
Engine version: Godot Engine v4.4.beta.custom_build (eee39f004b42a4948af90cdebedf65c34a4a4970)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[1] /lib64/libc.so.6(+0x1a050) [0x7f7351eaf050] (??:0)
[2] LocalVector<RenderingDeviceDriver::BufferTextureCopyRegion, unsigned int, false, false>::operator[](unsigned int) (/home/akien/Godot/godot/./core/templates/local_vector.h:178 (discriminator 2))
[3] RenderingDevice::texture_get_data_async(RID, unsigned int, Callable const&) (/home/akien/Godot/godot/./servers/rendering/rendering_device.cpp:2159 (discriminator 1))
[4] void call_with_validated_variant_args_ret_helper<__UnexistingClass, Error, RID, unsigned int, Callable const&, 0ul, 1ul, 2ul>(__UnexistingClass*, Error (__UnexistingClass::*)(RID, unsigned int, Callable const&), Variant const**, Variant*, IndexSequence<0ul, 1ul, 2ul>) (/home/akien/Godot/godot/./core/variant/binder_common.h:386 (discriminator 3))
[5] void call_with_validated_object_instance_args_ret<__UnexistingClass, Error, RID, unsigned int, Callable const&>(__UnexistingClass*, Error (__UnexistingClass::*)(RID, unsigned int, Callable const&), Variant const**, Variant*) (/home/akien/Godot/godot/./core/variant/binder_common.h:674)
[6] MethodBindTR<Error, RID, unsigned int, Callable const&>::validated_call(Object*, Variant const**, Variant*) const (/home/akien/Godot/godot/./core/object/method_bind.h:538)
[7] GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Callable::CallError&, GDScriptFunction::CallState*) (/home/akien/Godot/godot/./modules/gdscript/gdscript_vm.cpp:2247)
[8] GDScriptInstance::callp(StringName const&, Variant const**, int, Callable::CallError&) (/home/akien/Godot/godot/./modules/gdscript/gdscript.cpp:2073 (discriminator 1))
[9] Node::_gdvirtual__process_call(double) (/home/akien/Godot/godot/./scene/main/node.h:387 (discriminator 2))
[10] Node::_notification(int) (/home/akien/Godot/godot/./scene/main/node.cpp:56)
[11] Node::_notificationv(int, bool) (/home/akien/Godot/godot/./scene/main/node.h:49 (discriminator 14))
[12] Object::notification(int, bool) (/home/akien/Godot/godot/./core/object/object.cpp:914)
[13] SceneTree::_process_group(SceneTree::ProcessGroup*, bool) (/home/akien/Godot/godot/./scene/main/scene_tree.cpp:1060)
[14] SceneTree::_process(bool) (/home/akien/Godot/godot/./scene/main/scene_tree.cpp:1132 (discriminator 2))
[15] SceneTree::process(double) (/home/akien/Godot/godot/./scene/main/scene_tree.cpp:580)
[16] Main::iteration() (/home/akien/Godot/godot/main/main.cpp:4473 (discriminator 3))
[17] OS_LinuxBSD::run() (/home/akien/Godot/godot/platform/linuxbsd/os_linuxbsd.cpp:962 (discriminator 1))
[18] godot-git(main+0x14b) [0x66347f1] (/home/akien/Godot/godot/platform/linuxbsd/godot_linuxbsd.cpp:85)
[19] /lib64/libc.so.6(+0x3248) [0x7f7351e98248] (??:0)
[20] /lib64/libc.so.6(__libc_start_main+0x8b) [0x7f7351e9830b] (??:0)
[21] godot-git(_start+0x25) [0x66345e5] (??:?)
-- END OF BACKTRACE --
================================================================
```

It's only reproducible in 4.4+ as the API doesn't exist in 4.3. The method was introduced in 4.4.dev7 with #100110 and that version crashes too.

Spamming this function in `_process` definitely doesn't sound like a good idea and a supported use case, but it shouldn't crash.

CC @DarioSamo @clayjohn 
> Spamming this function in _process definitely doesn't sound like a good idea and a supported use case, but it shouldn't crash.

@akien-mga I'm working on NDI Output for Godot, so I need the texture every frame. ~~Is there a better place to call it from? Like on a draw notification/signal?~~ Now that you mention it, calling it every process doesn't make sense. The RenderingServer emits `frame_post_draw`. That'd be a better time to call the function.
This could be unrelated, but let's say I call `RD::texture_get_data_async` to request texture data, but in the meantime the Object/Node that owns the method pointed to by the Callable gets freed / deconstructed. Would that cause a crash? I think that's what I'm seeing here now that I'm calling it on `RenderingServer::frame_post_draw` instead of on every process.

```
request texture
receive texture
request texture
receive texture
deconstructed!
receive texture <-- note this

================================================================
CrashHandlerException: Program crashed
Engine version: Godot Engine v4.4.beta.custom_build (0b6a717ac17f1d6bd7963740cf2c2128b36ff7aa)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[0] <couldn't map PC to fn name>
[1] <couldn't map PC to fn name>
[2] <couldn't map PC to fn name>
[3] <couldn't map PC to fn name>
[4] CallableCustomExtension::call (C:\Users\Henry\Documents\godot\core\extension\gdextension_interface.cpp:173)
[5] Callable::callp (C:\Users\Henry\Documents\godot\core\variant\callable.cpp:58)
[6] CallableCustomBind::call (C:\Users\Henry\Documents\godot\core\variant\callable_bind.cpp:151)
[7] Callable::callp (C:\Users\Henry\Documents\godot\core\variant\callable.cpp:58)
[8] Callable::call<Vector<unsigned char> > (C:\Users\Henry\Documents\godot\core\variant\variant.h:961)
[9] RenderingDevice::_stall_for_frame (C:\Users\Henry\Documents\godot\servers\rendering\rendering_device.cpp:6575)
[10] RenderingDevice::_begin_frame (C:\Users\Henry\Documents\godot\servers\rendering\rendering_device.cpp:6353)
[11] RenderingDevice::swap_buffers (C:\Users\Henry\Documents\godot\servers\rendering\rendering_device.cpp:6223)
[12] RenderingServerDefault::_draw (C:\Users\Henry\Documents\godot\servers\rendering\rendering_server_default.cpp:91)
[13] RenderingServerDefault::draw (C:\Users\Henry\Documents\godot\servers\rendering\rendering_server_default.cpp:417)
[14] Main::iteration (C:\Users\Henry\Documents\godot\main\main.cpp:4490)
[15] OS_Windows::run (C:\Users\Henry\Documents\godot\platform\windows\os_windows.cpp:2062)
[16] widechar_main (C:\Users\Henry\Documents\godot\platform\windows\godot_windows.cpp:97)
[17] _main (C:\Users\Henry\Documents\godot\platform\windows\godot_windows.cpp:124)
[18] main (C:\Users\Henry\Documents\godot\platform\windows\godot_windows.cpp:136)
[19] __scrt_common_main_seh (D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
[20] <couldn't map PC to fn name>
-- END OF BACKTRACE --
================================================================
```

> This could be unrelated, but let's say I call `RD::texture_get_data_async` to request texture data, but in the meantime the Object/Node that owns the method pointed to by the Callable gets freed / deconstructed. Would that cause a crash? I think that's what I'm seeing here now that I'm calling it on `RenderingServer::frame_post_draw` instead of on every process.

That'd likely lead to a crash yes. You're responsible for the lifetime of your objects and you shouldn't destroy them until the callback comes back.

You could easily work around that by handing off that responsibility to an object that always lives and is in charge of redirecting the data to the object.

That said, I'll still investigate the original stack trace to see if it can be reproduced in the situation that was mentioned in the OP first.

Fix patch:
diff --git a/servers/rendering/rendering_device.cpp b/servers/rendering/rendering_device.cpp
index 9716a42c97b9..055fcbeda64f 100644
--- a/servers/rendering/rendering_device.cpp
+++ b/servers/rendering/rendering_device.cpp
@@ -721,7 +721,6 @@ Error RenderingDevice::buffer_get_data_async(RID p_buffer, const Callable &p_cal
 	_check_transfer_worker_buffer(buffer);
 
 	BufferGetDataRequest get_data_request;
-	uint32_t flushed_copies = 0;
 	get_data_request.callback = p_callback;
 	get_data_request.frame_local_index = frames[frame].download_buffer_copy_regions.size();
 	get_data_request.size = p_size;
@@ -738,22 +737,26 @@ Error RenderingDevice::buffer_get_data_async(RID p_buffer, const Callable &p_cal
 			return err;
 		}
 
-		if ((get_data_request.frame_local_count > 0) && required_action == STAGING_REQUIRED_ACTION_FLUSH_AND_STALL_ALL) {
+		const bool flush_frames = (get_data_request.frame_local_count > 0) && required_action == STAGING_REQUIRED_ACTION_FLUSH_AND_STALL_ALL;
+		if (flush_frames) {
 			if (_buffer_make_mutable(buffer, p_buffer)) {
 				// The buffer must be mutable to be used as a copy source.
 				draw_graph.add_synchronization();
 			}
 
-			for (uint32_t i = flushed_copies; i < get_data_request.frame_local_count; i++) {
+			for (uint32_t i = 0; i < get_data_request.frame_local_count; i++) {
 				uint32_t local_index = get_data_request.frame_local_index + i;
 				draw_graph.add_buffer_get_data(buffer->driver_id, buffer->draw_tracker, frames[frame].download_buffer_staging_buffers[local_index], frames[frame].download_buffer_copy_regions[local_index]);
 			}
-
-			flushed_copies = get_data_request.frame_local_count;
 		}
 
 		_staging_buffer_execute_required_action(download_staging_buffers, required_action);
 
+		if (flush_frames) {
+			get_data_request.frame_local_count = 0;
+			get_data_request.frame_local_index = frames[frame].download_buffer_copy_regions.size();
+		}
+
 		RDD::BufferCopyRegion region;
 		region.src_offset = submit_from + p_offset;
 		region.dst_offset = block_write_offset;
@@ -775,7 +778,7 @@ Error RenderingDevice::buffer_get_data_async(RID p_buffer, const Callable &p_cal
 			draw_graph.add_synchronization();
 		}
 
-		for (uint32_t i = flushed_copies; i < get_data_request.frame_local_count; i++) {
+		for (uint32_t i = 0; i < get_data_request.frame_local_count; i++) {
 			uint32_t local_index = get_data_request.frame_local_index + i;
 			draw_graph.add_buffer_get_data(buffer->driver_id, buffer->draw_tracker, frames[frame].download_buffer_staging_buffers[local_index], frames[frame].download_buffer_copy_regions[local_index]);
 		}
@@ -2098,7 +2101,6 @@ Error RenderingDevice::texture_get_data_async(RID p_texture, uint32_t p_layer, c
 	uint32_t block_write_offset;
 	uint32_t block_write_amount;
 	StagingRequiredAction required_action;
-	uint32_t flushed_copies = 0;
 	for (uint32_t i = 0; i < tex->mipmaps; i++) {
 		uint32_t image_total = get_image_format_required_size(tex->format, tex->width, tex->height, tex->depth, i + 1, &w, &h, &d);
 		uint32_t tight_mip_size = image_total - mipmap_offset;
@@ -2119,17 +2121,21 @@ Error RenderingDevice::texture_get_data_async(RID p_texture, uint32_t p_layer, c
 					Error err = _staging_buffer_allocate(download_staging_buffers, to_allocate, required_align, block_write_offset, block_write_amount, required_action, false);
 					ERR_FAIL_COND_V(err, ERR_CANT_CREATE);
 
-					if ((get_data_request.frame_local_count > 0) && required_action == STAGING_REQUIRED_ACTION_FLUSH_AND_STALL_ALL) {
-						for (uint32_t j = flushed_copies; j < get_data_request.frame_local_count; j++) {
+					const bool flush_frames = (get_data_request.frame_local_count > 0) && required_action == STAGING_REQUIRED_ACTION_FLUSH_AND_STALL_ALL;
+					if (flush_frames) {
+						for (uint32_t j = 0; j < get_data_request.frame_local_count; j++) {
 							uint32_t local_index = get_data_request.frame_local_index + j;
 							draw_graph.add_texture_get_data(tex->driver_id, tex->draw_tracker, frames[frame].download_texture_staging_buffers[local_index], frames[frame].download_buffer_texture_copy_regions[local_index]);
 						}
-
-						flushed_copies = get_data_request.frame_local_count;
 					}
 
 					_staging_buffer_execute_required_action(download_staging_buffers, required_action);
 
+					if (flush_frames) {
+						get_data_request.frame_local_count = 0;
+						get_data_request.frame_local_index = frames[frame].download_buffer_texture_copy_regions.size();
+					}
+
 					RDD::BufferTextureCopyRegion copy_region;
 					copy_region.buffer_offset = block_write_offset;
 					copy_region.texture_subresources.aspect = tex->read_aspect_flags;
@@ -2154,12 +2160,11 @@ Error RenderingDevice::texture_get_data_async(RID p_texture, uint32_t p_layer, c
 	}
 
 	if (get_data_request.frame_local_count > 0) {
-		for (uint32_t i = flushed_copies; i < get_data_request.frame_local_count; i++) {
+		for (uint32_t i = 0; i < get_data_request.frame_local_count; i++) {
 			uint32_t local_index = get_data_request.frame_local_index + i;
 			draw_graph.add_texture_get_data(tex->driver_id, tex->draw_tracker, frames[frame].download_texture_staging_buffers[local_index], frames[frame].download_buffer_texture_copy_regions[local_index]);
 		}
 
-		flushed_copies = get_data_request.frame_local_count;
 		frames[frame].download_texture_get_data_requests.push_back(get_data_request);
 	}
 


Test patch: