Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash when freeing Node with invalid owner #78736

Closed
Haydoggo opened this issue Jun 27, 2023 · 2 comments · Fixed by #78997
Closed

Crash when freeing Node with invalid owner #78736

Haydoggo opened this issue Jun 27, 2023 · 2 comments · Fixed by #78997

Comments

@Haydoggo
Copy link
Contributor

Haydoggo commented Jun 27, 2023

Godot version

v4.0.3.stable.official [5222a99]

System information

Godot v4.0.3.stable - Windows 10.0.19045 - Vulkan (Forward+) - dedicated NVIDIA GeForce GTX 1080 (NVIDIA; 31.0.15.1694) - Intel(R) Core(TM) i5-10600KF CPU @ 4.10GHz (12 Threads)

Issue description

Freeing a node after freeing its owner sometimes inconsistently causes a crash, and other times throws the following error:

erase: Condition "p_I->data != this" is true. Returning: false
  <C++ Source>   ./core/templates/list.h:223 @ erase()

Old description:

Calling `queue_free()` in certain conditions cause the following error to be thrown, and occasionally cause errorless crashes.
erase: Condition "p_I->data != this" is true. Returning: false ./core/templates/list.h:223 @ erase()
The exact conditions aren't clear: commenting out almost any line or awaiting a frame almost anywhere prevents the error, so maybe some there's some race condition?

The error also isn't super consistent, but I've managed to create a minimal reproduction project that causes the error more >50% of the time.

Call stack on crash:

================================================================
CrashHandlerException: Program crashed
Engine version: Godot Engine v4.0.3.stable.custom_build (5222a99f5d38cd5346254cefed8f65315bca4fcb)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[0] List<Node *,DefaultAllocator>::_Data::erase (C:\Users\Hayden\Documents\Godot\Source\godot\core\templates\list.h:238)
[1] List<Node *,DefaultAllocator>::_Data::erase (C:\Users\Hayden\Documents\Godot\Source\godot\core\templates\list.h:238)
[2] List<Node *,DefaultAllocator>::erase (C:\Users\Hayden\Documents\Godot\Source\godot\core\templates\list.h:429)
[3] Node::_propagate_after_exit_tree (C:\Users\Hayden\Documents\Godot\Source\godot\scene\main\node.cpp:265)
[4] Node::remove_child (C:\Users\Hayden\Documents\Godot\Source\godot\scene\main\node.cpp:1227)
[5] Node::_notification (C:\Users\Hayden\Documents\Godot\Source\godot\scene\main\node.cpp:168)
[6] Node::_notificationv (C:\Users\Hayden\Documents\Godot\Source\godot\scene\main\node.h:46)
[7] Object::notification (C:\Users\Hayden\Documents\Godot\Source\godot\core\object\object.cpp:792)
[8] Object::_predelete (C:\Users\Hayden\Documents\Godot\Source\godot\core\object\object.cpp:197)
[9] predelete_handler (C:\Users\Hayden\Documents\Godot\Source\godot\core\object\object.cpp:1826)
[10] memdelete<Object> (C:\Users\Hayden\Documents\Godot\Source\godot\core\os\memory.h:105)
[11] SceneTree::_flush_delete_queue (C:\Users\Hayden\Documents\Godot\Source\godot\scene\main\scene_tree.cpp:1065)
[12] SceneTree::process (C:\Users\Hayden\Documents\Godot\Source\godot\scene\main\scene_tree.cpp:476)
[13] Main::iteration (C:\Users\Hayden\Documents\Godot\Source\godot\main\main.cpp:3169)
[14] OS_Windows::run (C:\Users\Hayden\Documents\Godot\Source\godot\platform\windows\os_windows.cpp:1295)
[15] widechar_main (C:\Users\Hayden\Documents\Godot\Source\godot\platform\windows\godot_windows.cpp:181)
[16] _main (C:\Users\Hayden\Documents\Godot\Source\godot\platform\windows\godot_windows.cpp:203)
[17] main (C:\Users\Hayden\Documents\Godot\Source\godot\platform\windows\godot_windows.cpp:217)
[18] WinMain (C:\Users\Hayden\Documents\Godot\Source\godot\platform\windows\godot_windows.cpp:231)
[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 --
================================================================

Steps to reproduce

EDIT: Made a simpler single script MRP:

extends Node

func _process(delta: float) -> void:
	var owner_node = Node.new()
	var child_node = Node.new()
	
	owner_node.add_child(child_node)
	child_node.owner = owner_node
	child_node.reparent(self)
	
	owner_node.queue_free()
	await get_tree().process_frame
	child_node.queue_free()

Old method:
Simply run the provided MRP. The error doesn't show up every time it is run, nor does it cause a crash everytime, so you may need to run it more than once.

While most of my testing was done in 4.0.3, this still occurs in 4.1 beta 3 as well

Minimal reproduction project

MRP.zip

@kleonc
Copy link
Member

kleonc commented Jun 27, 2023

The issue is Node.owner pointing to an already freed instance. In:

extends Button

var target_node : Node

func _process(delta: float) -> void:
	if target_node:
		target_node.queue_free()
	var new_node = preload("res://part.tscn").instantiate()
	target_node = new_node.get_child(0)
	target_node.get_parent().remove_child(target_node)
	add_child(target_node)
	new_node.queue_free()

the target_node.owner will remain pointing at the new_node and after new_node is freed such no longer valid owner pointer is not cleared/nulled properly. Doing so fixes the crash:

extends Button

var target_node : Node

func _process(delta: float) -> void:
	if target_node:
		target_node.queue_free()
	var new_node = preload("res://part.tscn").instantiate()
	target_node = new_node.get_child(0)
	target_node.get_parent().remove_child(target_node)
	target_node.owner = null
	add_child(target_node)
	new_node.queue_free()

But indeed it shouldn't be crashing, invalid owner pointer should be cleared/nulled internally at some point.

@Haydoggo
Copy link
Contributor Author

Ah that explains why this only occurs if Part is instantiated from a PackedScene and not if it's constructed via Node.new and add_child

@Haydoggo Haydoggo changed the title Errors and Crash with queue_free() in certain conditions Crash when freeing Node with invalid owner Jun 28, 2023
@akien-mga akien-mga added this to the 4.3 milestone Nov 24, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants