-
-
Notifications
You must be signed in to change notification settings - Fork 548
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
Node pointers can't be passed into UtilityFunctions::is_instance_valid() to check if they have not been freed; is something needed for this? #1390
Comments
In a C++ module, rather than using Regarding fixing There is no way to check if an |
After over a year of using My last bit of code in the callstack is:
If it doesn't work, and isn't going to work, then I suggest that you not expose it and in the gdextension documentation (👀?) say how to check objects so we don't have to use Issues and troubleshooting to figure things out. I assume that if there's a function available that it's safe to use. Here's an example usage of the described solution and a replacement // Additional instance id tracker
Camera3D *_camera = nullptr;
uint64_t _camera_instance_id = 0;
// Save instance id when setting object
void set_camera(Camera3D *p_camera) {
_camera = p_camera;
if (p_camera) {
_camera_instance_id = _camera->get_instance_id();
} else {
_camera_instance_id = 0;
}
}
// Check just the instance ID, or both your variables:
is_instance_valid(_camera_instance_id );
is_instance_valid(_camera_instance_id , _camera);
_FORCE_INLINE_ bool is_instance_valid(uint64_t p_instance_id, Object *p_object = nullptr) {
Object *obj = ObjectDB::get_instance(p_instance_id);
if (p_object != nullptr) {
return p_instance_id> 0 && p_object == obj;
} else {
return p_instance_id> 0 && obj != nullptr;
}
} |
I agree, exposing this function is a trap. I've just posted PR #1513 to unexpose it |
Godot version
4.2.1.stable
godot-cpp version
4.2.1.stable
System information
Any
Issue description
Currently,
is_instance_valid()
dereferences object pointers passed to it due to implicit cast to Variant. As such, executingis_instance_valid()
on freed nodes, unlike in gdscript, causes a use-after-free crash. Currently, there doesn't seem to be a way to store nodes where UtilityFunctions::is_instance_valid() can work without sacrificing typing (by storing everything as Variant), and the example project simply uses pointers for passing around object pointers as well.Steps to reproduce
Call
UtilityFunctions::is_instance_valid()
on a freed node pointer. The program crashes instead of returning false.Minimal reproduction project
See steps to reproduce.
The text was updated successfully, but these errors were encountered: