Runtime error when passing a previously freed Object as a statically typed function argument [GDScript]
Tested versions
Reproducible in: v4.2.1.stable.official [b09f793f5]
System information
Windows 10
Issue description
If you pass a reference of a previously freed instance of an Object as a statically typed function parameter, we get a CALL_ERROR_INVALID_ARGUMENT error.
In some similiar cases, the same issue does not occur:
- Returning a freed instance from a function with static return type is OK
- Initializing a statically typed variable using a freed instance is OK
- Passing a freed instance as a Variant function argument is OK
Expected result: The reference to a freed object is passed to the function without issue.
Additional information
The error occurs here, because the is_type function returns false when the target Object was previously freed:
https://github.com/godotengine/godot/blob/b94eb58d35b3dd8a9f522bc90df0db73862ef326/modules/gdscript/gdscript_vm.cpp#L514-L520
Adding a special case check like this to the above code snippet will solve the problem, but there is probably a better solution:
if (!argument_types[i].is_type(*p_args[i], true)) {
if (p_args[i]->get_type() == Variant::OBJECT) {
bool was_freed = false;
p_args[i]->get_validated_object_with_check(was_freed);
if (was_freed) {
memnew_placement(&stack[i + 3], Variant(*p_args[i]));
continue;
}
}
// ...
}
Steps to reproduce
func _ready() -> void:
var n := Node.new()
n.free()
test(n)
func test(n: Node) -> void:
pass
Create a new scene, attach the above script, and run the scene.
Minimal reproduction project (MRP)
I think this issue should also receive a bug tag for the following reasons:
Even though the is_type function returning false in this case seems to be intentional, the fact that this causes an error at the script function call level is inconsistent with the other similar cases I mentioned.
More importantly, having seemingly benign script code cause a runtime error definitely sounds like a bug, which can also happen pretty easily if someone is using static typing in their code extensively. (For example, if you hold a reference to a Node in one of your scripts and call a function every frame, like func move_towards(target: Node), and said node gets freed at some point, a runtime error occurs on the function call.)
Well, that's what is to discuss 🙂, I'd say this isn't necessarily clear cut, a freed object isn't the same as a null, so it's unclear what's wrong here, I'm all for considering this a bug but I don't find it clear, hence the label
There's a genuine question if a freed object is the type it used to hold or not
One interesting thing I found is with lambda captures. In this case, there is code to specifically pass in "null" instead of the freed instance, probably in order to avoid the runtime error from the function call:
https://github.com/godotengine/godot/blob/d5064a7d443d4b335169c0d02e822ee720abcc5d/modules/gdscript/gdscript_lambda_callable.cpp#L95C1-L103
The reason why the issue does not occur in the case of assigning a freed instance to a statically typed variable, is because if the types match at compile time, the bytecode generator will generate an OPCODE_ASSIGN, which does no type checking. This is of course not true, if one of the variables is a Variant, which means the following code WILL produce a similiar runtime error mentioned in the issue:
var n : Variant = Node.new()
n.free()
var n2: Node = n # error
I was incorrect to assume that this error only occurs with function arguments, this can actually occur at any time there is runtime type checking.
I've run into a similar situation with lambda captures that creates unavoidable errors:
func _ready():
var x = Object.new()
var f = func():
print(is_instance_valid(x))
pass
x.free()
f.call()
It is impossible to even check whether the value is valid inside the lambda without producing Lambda capture at index 0 was freed. Passed "null" instead. as an error.
edit: v4.2.stable.mono.official [46dc27791]
I've run into a similar situation with lambda captures that creates unavoidable errors
I've also run in your same problem when I ported the project to v4.2.1.stable.official [b09f793f5]. I was creating a lambda and then calling the call_deferred method on it:
(func(): dictionary.erase(x)).call_deferred()
Changing the lambda to a private function fixed the error.
There's a genuine question if a freed object is the type it used to hold or not
Because in GDScript you hold and pass references to objects, i think simply freeing the underlying memory should not invalidate the type of the reference you are holding. You are still pointing to the same type of thing, the memory is just not valid anymore, which can be handled by user code.
The more interesting case is when the object you are referencing actually changed type, and then freed. In this case, it would be appropriate to throw a type checking error when you try to use this reference with the old type. However, this would require GDScript to store information about the freed instance (like what type it used to be).
I have the same problem. I have too little insight into the core to give any valuable input, however what I'd request is a way to see where the source of the error is in my GDScript code.
Because now I only get this...
E 0:00:44:0391 call: Lambda capture at index 0 was freed. Passed "null" instead.
<C++ Source> modules\gdscript\gdscript_lambda_callable.cpp:99 @ call()
without any information on what GDScript code called this, and thus I cannot fix my mistake.