godot: Editor crashes using custom property inspector with custom resource

Godot version

v4.0.beta9.official.e780dc332

System information

Ubuntu 20.04.3 LTS, Vulkan, TU116 [GeForce GTX 1660 Ti], nvidia 470.161.03

Issue description

Given a custom property inspector which handles a property of a custom resource type, the method get_edited_object() might return null in some cases. Especially while creating a new resource of the given type

https://user-images.githubusercontent.com/9679864/208495343-d39fab46-c143-4af6-8b6c-46a608ba5344.mp4

================================================================
handle_crash: Program crashed with signal 11
Engine version: Godot Engine v4.0.beta9.official (e780dc332a0a3f642a6daf8548cb211d79a2cc45)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[1] /lib/x86_64-linux-gnu/libc.so.6(+0x430c0) [0x7f573c6420c0] (??:0)
[2] /home/benjamin/Programme/Godot_v4.0-beta9_linux.x86_64() [0x44dad85] (??:0)
[3] /home/benjamin/Programme/Godot_v4.0-beta9_linux.x86_64() [0x43df92c] (??:0)
[4] /home/benjamin/Programme/Godot_v4.0-beta9_linux.x86_64() [0x43dfd60] (??:0)
[5] /home/benjamin/Programme/Godot_v4.0-beta9_linux.x86_64() [0x418fd74] (??:0)
[6] /home/benjamin/Programme/Godot_v4.0-beta9_linux.x86_64() [0x1c15e86] (??:0)
[7] /home/benjamin/Programme/Godot_v4.0-beta9_linux.x86_64() [0x1c2eec5] (??:0)
[8] /home/benjamin/Programme/Godot_v4.0-beta9_linux.x86_64() [0x1c59f2a] (??:0)
[9] /home/benjamin/Programme/Godot_v4.0-beta9_linux.x86_64() [0x44118da] (??:0)
[10] /home/benjamin/Programme/Godot_v4.0-beta9_linux.x86_64() [0x1b17c4e] (??:0)
[11] /home/benjamin/Programme/Godot_v4.0-beta9_linux.x86_64() [0x44118da] (??:0)
[12] /home/benjamin/Programme/Godot_v4.0-beta9_linux.x86_64() [0x2b8f98b] (??:0)
[13] /home/benjamin/Programme/Godot_v4.0-beta9_linux.x86_64() [0x44118da] (??:0)
[14] /home/benjamin/Programme/Godot_v4.0-beta9_linux.x86_64() [0x2b1efc9] (??:0)
[15] /home/benjamin/Programme/Godot_v4.0-beta9_linux.x86_64() [0x2b1f7dc] (??:0)
[16] /home/benjamin/Programme/Godot_v4.0-beta9_linux.x86_64() [0x2b1fb1a] (??:0)
[17] /home/benjamin/Programme/Godot_v4.0-beta9_linux.x86_64() [0x2a936f5] (??:0)
[18] /home/benjamin/Programme/Godot_v4.0-beta9_linux.x86_64() [0x2ae7075] (??:0)
[19] /home/benjamin/Programme/Godot_v4.0-beta9_linux.x86_64() [0x2aeb401] (??:0)
[20] /home/benjamin/Programme/Godot_v4.0-beta9_linux.x86_64() [0x2aeb9db] (??:0)
[21] /home/benjamin/Programme/Godot_v4.0-beta9_linux.x86_64() [0x2a99645] (??:0)
[22] /home/benjamin/Programme/Godot_v4.0-beta9_linux.x86_64() [0xe3b404] (??:0)
[23] /home/benjamin/Programme/Godot_v4.0-beta9_linux.x86_64() [0x41b3e77] (??:0)
[24] /home/benjamin/Programme/Godot_v4.0-beta9_linux.x86_64() [0x41b4cf5] (??:0)
[25] /home/benjamin/Programme/Godot_v4.0-beta9_linux.x86_64() [0xe3e20d] (??:0)
[26] /home/benjamin/Programme/Godot_v4.0-beta9_linux.x86_64() [0xdba3e2] (??:0)
[27] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf3) [0x7f573c6230b3] (??:0)
[28] /home/benjamin/Programme/Godot_v4.0-beta9_linux.x86_64() [0xddbc1e] (??:0)
-- END OF BACKTRACE --
================================================================

Steps to reproduce

Using reproduction project

Load project
Create new resource of type CustomResource
Crash

From scratch

Create a plugin with a custom property inspector (basic text editor for example)
Create a custom resource type with an exported property which should use the custom inspector
Create a resource of the new resource type
The plugin _update_property() routine will be called and get_edited_object() might return null sometimes

Minimal reproduction project

Project.zip

About this issue

Original URL
State: closed
Created 2 years ago
Comments: 18 (17 by maintainers)

Most upvoted comments

I think GDScript assumes the refcount is incremented by the native function

I’ve been debugging the issue and I agree with that hypothesis.

So it seems there are cases where it should assume and some others where it shoudn’t.

6141ce2cb8ae54965190257a1ffd6a6abd02aaea made it behave right (or maye not?) in cases where it should assume, so the ref count is not extraneously incremented.

However, that change makes things go wrong in cases where it shouldn’t assume.

RandomShaper on Dec 20, 2022

Looking closer at the issue, it looks like calling get_edited_object is decrementing the ref count? I can’t think of any other way you’d get this output:

1. _update_property entered. object: <Resource#-9223370278518594817>
2. _update_property entered. object: <Resource#-9223370278518594817>
3. _update_property entered. object: <Freed Object>
4. _update_property entered. object: <Freed Object>
5. _update_property entered. object: <Freed Object>

I’m not sure this is what’s happening, but I think GDScript assumes the refcount is incremented by the native function (expecting it to look like: Ref<RefCounted> func() { rather than Object* func() {). When the GDScript interpreter is done with the value returned from get_edited_object, the refcount is decremented.

https://github.com/godotengine/godot/blob/2b056115ef6c0e92eac70692d83676ea6e9456da/editor/editor_inspector.cpp#L409-L411

This looks awfully similar to the cases that #44980 was fixing.

Maybe get_edited_object and its usage should be updated.

- Object *EditorProperty::get_edited_object() {
- 	return object;
}
+ Variant EditorProperty::get_edited_object() {
+ 	return Variant(object);

But I don’t think this is the correct long term solution. If any function that returns Object* is capable of returning a RefCounted* without incrementing the counter (super brittle!), then something needs to change or we’ll see this bug again. Either: A. returning Object* shouldn’t be allowed, replace with Variant (and gdscript never manually increments refcount), or B. GDScript needs somehow know or infer whether the refcount was incremented already. (no clue how)

briansemrau on Dec 20, 2022

OK, makes sense. We may also want to revert the changes from Object * to Variant that were made solely for that. I’ll make a PR and ping you somehow.

RandomShaper on Feb 2, 2023

I tried to bisect and it seems that commit 6141ce2cb8ae54965190257a1ffd6a6abd02aaea could be the reason. But I’m not sure 100% (first time bisect)

BenjaminSchulte on Dec 20, 2022