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

Creating two mutually recursive dictionaries freezes Godot and spams console with endless errors. #80990

Open
ghost opened this issue Aug 25, 2023 · 4 comments

Comments

@ghost
Copy link

ghost commented Aug 25, 2023

Godot version

v4.2.dev3.official.013e8e3af

System information

Windows 10.0.19045 - GLES3 (Compatibility)

Issue description

Running the following code freezes a Godot project and causes the error:

ERROR: Max recursion reached
   at: recursive_hash (core/variant/dictionary.cpp:267)

To start printing infinitely many times into the console:

func _ready():
	var test1 = {}
	var test2 = {}
	test1[test2] = test2
	test2[test1] = test1

Steps to reproduce

Run the code:

func _ready():
	var test1 = {}
	var test2 = {}
	test1[test2] = test2
	test2[test1] = test1

Minimal reproduction project

func _ready():
	var test1 = {}
	var test2 = {}
	test1[test2] = test2
	test2[test1] = test1
@ajreckof
Copy link
Member

ajreckof commented Aug 25, 2023

This is most likely recursive hash not achieving to get a proper hash since it is infinite.
Be aware

func _ready() -> void:
	var dict = {}
	dict[dict] = dict
	print({dict : 3})

will reproduce the same thing but the same with array will log a max recursion reached but will NOT freeze

func _ready() -> void:
	var array = []
	array.push_back(array)
	print({array : 3})

I think the problem is that even if it reasches max recursion since it has two thing to check for each recursion it has to reach 2 power the max recursion limit times the max recursion which is why it freeze.

However there is something fundamentally strange with how arrray/dict are handled as dictionnaries key. hash are calculated recursively but array/dict can be modified after having been set as keys which means the hash will have been changed and the value will not be able to be found anymore. hash should either be per instance like objects or array/dict should be constants. Either of thos solution will also fix the bug reported here.
To see what I'm talking about here is a small snippet exhibiting the problem I'm talking about

var dict = {}
var dict2 = {}
dict[dict2] = 3
print(dict)
dict2[2] = 5
print(dict)

Which will print

{ {  }: 3 }
{ { 2: 5 }: <null> }

@ghost
Copy link
Author

ghost commented Aug 25, 2023

Related: #79626 (Mutating a mutable dictionary key causes weird behaviour (instead of an error)).

@dalexeev
Copy link
Member

I'm not sure if this is a bug or a reasonable limitation of OrderedHashMap. There is an old draft PR #44251 that seems to be trying to remove this limitation, but I can't say anything about this code.

As for the "Max recursion reached" error, it's a guard condition. I recently made a PR #80888 that backports the recursive hash and the check to 3.x.

@dalexeev dalexeev added the bug label Aug 25, 2023
@ghost
Copy link
Author

ghost commented Aug 25, 2023

The problem is that the guard condition does not work properly in 4.2.dev3 I believe.
As you can see below, running my reproduction code makes the error message print recursively:
It probably should print just once instead.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants