Closed allenwp closed 2 months ago
I'm guessing this is how it's supposed to work, as it's not supposed to un-allocate the memory. It also says this in the documentation, that until the game needs the memory, it won't delete the objects.
"Objects" in "Monitors" panel shows the number of objects growing indefinitely.
Changing var obj: MyObj
inside MyObj
to var obj: WeakRef
fixes the problem.
Assigning the weak reference to a plain MyObj
directly is an error:
var x: MyObj = weakref(obj_2)
# ERROR: Trying to assign value of type 'WeakRef' to a variable of type ''.
I guess something wrong happened during the implicit conversion.
Use NOTIFICATION_PREDELETE
to check if the object is freed:
class MyObj:
var id: int
var obj: MyObj
func _init(p_id: int) -> void:
id = p_id
func _notification(what: int) -> void:
if what == NOTIFICATION_PREDELETE:
prints(id, "freed")
func _run() -> void:
var obj_1 = MyObj.new(1)
var obj_2 = MyObj.new(2)
#obj_1.obj = obj_2 # Does not leak.
obj_1.obj = weakref(obj_2) # Does not leak.
class MyObj:
var id: int
var obj: WeakRef # <-- !!!
func _init(p_id: int) -> void:
id = p_id
func _notification(what: int) -> void:
if what == NOTIFICATION_PREDELETE:
prints(id, "freed")
func _run() -> void:
var obj_1 = MyObj.new(1)
var obj_2 = MyObj.new(2)
obj_1.obj = obj_2 # `obj_2` leaks.
#obj_1.obj = weakref(obj_2) # Does not leak.
The implicit conversion between WeakRef
and Object
/RefCounted
is quite unexpected. But I think this is not a GDScript issue, the line is marked as unsafe (though we don't have the the variables are untyped. Looks like this is a core issue that happens at runtime.UNSAFE_ASSIGNMENT
warning)
There's no casting at least not valid casting occurring, try just print(obj_1.obj)
after assigning it, it'll print <null>
, there's no warning as weakref
is just Variant
and can be cast to any object type as null
The leak in the obj -> WeakRef
though is strange, that does sound like it's something in the GDScript side though
Edit: It happens regardless of what's contained in obj
, so it seems to be related to some leak on the stack somehow, where the local variable is lost in the process and the underlying variant isn't freed, you can do:
obj_1.obj = obj_2
obj_1.obj = weakref(obj_2)
And it still happens
Edit 2: This seems to be related to how property access is handled, as:
var w : WeakRef = obj_2
Fails with an error at runtime, so something is wrong with how obj_1.obj
is processed on assignment, possibly causing some leak in the associated argument data
Will try dig into the casting or conversion of refcounted, seems to be some glitch with the count increment:
print(obj_2.get_reference_count()) # Prints 1
obj_1.obj = obj_2
print(obj_2.get_reference_count()) # Prints 2!
This seems to be related to how property access is handled, as:
var w : WeakRef = obj_2
Fails with an error at runtime, so something is wrong with how
obj_1.obj
is processed on assignment
If the base type is not known at compile time, then the property is assigned as follows:
Variant::set_named()
-> Object::set()
-> GDScriptInstance::set()
And Object.set()
does not report errors, according to the docs:
Assigns value to the given property. If the property does not exist or the given value's type doesn't match, nothing happens.
The code that's causing the leak is this: https://github.com/godotengine/godot/blob/29b3d9e9e538f0aa8effc8ad8bf19a2915292a89/modules/gdscript/gdscript.cpp#L1605-L1612
Something about how it handles conversions here means the value leaks, even though at the end of the conversion value
is null
Can't quite find what in the code is broken exactly, probably something in: https://github.com/godotengine/godot/blob/29b3d9e9e538f0aa8effc8ad8bf19a2915292a89/core/variant/variant_construct.cpp#L331-L348
Not handling something correctly
It seems that it's due to how the construction of an object doesn't correctly assign type, so it's a Variant
with type NIL
but the object is still assigned
Will write up a fix, new changes makes it error on runtime, and doesn't leak
Tested versions
System information
Godot v4.2.1.stable - Windows 10.0.19045 - Vulkan (Mobile) - dedicated NVIDIA GeForce GTX 980 Ti (NVIDIA; 31.0.15.3699) - Intel(R) Core(TM) i7-6700K CPU @ 4.00GHz (8 Threads)
Issue description
The following code leaks memory:
If you swap the commented line with the line that leaks, it will no longer leak.
This also happens with other types:
I expect that
weakref()
should not have this sort of impact in this scenario. Please correct me if I am mistaken!With
weakref()
:Without
weakref()
:Steps to reproduce
Minimal reproduction project (MRP)
weakref-leak-bug.zip