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

Executing PluginScript.new().instance_has(BoxShape.new()) crashes Godot #46020

Closed
qarmin opened this issue Feb 14, 2021 · 2 comments · Fixed by #49191
Closed

Executing PluginScript.new().instance_has(BoxShape.new()) crashes Godot #46020

qarmin opened this issue Feb 14, 2021 · 2 comments · Fixed by #49191

Comments

@qarmin
Copy link
Contributor

qarmin commented Feb 14, 2021

Godot version:
Godot 3.2.4 rc 2

Issue description:
Executing

PluginScript.new().instance_has(BoxShape.new())

crashes with backtrace

modules/gdnative/pluginscript/pluginscript_script.cpp:207:17: runtime error: member call on null pointer of type 'struct PluginScriptLanguage'
modules/gdnative/pluginscript/pluginscript_script.cpp:207:17: runtime error: member access within null pointer of type 'struct PluginScriptLanguage'
handle_crash: Program crashed with signal 11
Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[1] godots() [0x16f757a] (/mnt/Miecz/godot3.2/platform/x11/crash_handler_x11.cpp:54)
[2] /lib/x86_64-linux-gnu/libc.so.6(+0x46210) [0x7f81438fe210] (??:0)
[3] PluginScript::instance_has(Object const*) const (/mnt/Miecz/godot3.2/modules/gdnative/pluginscript/pluginscript_script.cpp:207)
[4] MethodBind1RC<bool, Object const*>::call(Object*, Variant const**, int, Variant::CallError&) (/mnt/Miecz/godot3.2/./core/method_bind.gen.inc:1333 (discriminator 8))
[5] Object::call(StringName const&, Variant const**, int, Variant::CallError&) (/mnt/Miecz/godot3.2/core/object.cpp:919 (discriminator 1))
[6] Variant::call_ptr(StringName const&, Variant const**, int, Variant*, Variant::CallError&) (/mnt/Miecz/godot3.2/core/variant_call.cpp:1129 (discriminator 1))
[7] GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Variant::CallError&, GDScriptFunction::CallState*) (/mnt/Miecz/godot3.2/modules/gdscript/gdscript_function.cpp:1089)
[8] GDScriptInstance::_ml_call_reversed(GDScript*, StringName const&, Variant const**, int) (/mnt/Miecz/godot3.2/modules/gdscript/gdscript.cpp:1269)
[9] GDScriptInstance::call_multilevel_reversed(StringName const&, Variant const**, int) (/mnt/Miecz/godot3.2/modules/gdscript/gdscript.cpp:1278)
[10] Node::_notification(int) (/mnt/Miecz/godot3.2/scene/main/node.cpp:152)
[11] Node::_notificationv(int, bool) (/mnt/Miecz/godot3.2/./scene/main/node.h:46 (discriminator 14))
[12] CanvasItem::_notificationv(int, bool) (/mnt/Miecz/godot3.2/./scene/2d/canvas_item.h:166 (discriminator 3))
[13] Node2D::_notificationv(int, bool) (/mnt/Miecz/godot3.2/./scene/2d/node_2d.h:38 (discriminator 3))
[14] Object::notification(int, bool) (/mnt/Miecz/godot3.2/core/object.cpp:931)
[15] Node::_propagate_ready() (/mnt/Miecz/godot3.2/scene/main/node.cpp:197)
[16] Node::_propagate_ready() (/mnt/Miecz/godot3.2/scene/main/node.cpp:186 (discriminator 2))
[17] Node::_set_tree(SceneTree*) (/mnt/Miecz/godot3.2/scene/main/node.cpp:2560)
[18] SceneTree::init() (/mnt/Miecz/godot3.2/scene/main/scene_tree.cpp:464)
[19] OS_X11::run() (/mnt/Miecz/godot3.2/platform/x11/os_x11.cpp:3628)
[20] godots(main+0x331) [0x16ee467] (/mnt/Miecz/godot3.2/platform/x11/godot_x11.cpp:57)
[21] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf3) [0x7f81438df0b3] (??:0)
[22] godots(_start+0x2e) [0x16ee07e] (??:?)
@pfertyk
Copy link
Contributor

pfertyk commented May 29, 2021

I'd like to submit a fix. The PR is to master, but it should be easy to cherry-pick to 3.x. The only difference is that BoxShape was renamed to BoxShape3D, but that doesn't affect the fix :)

The problem was that _language is null when we invoke instance_has. I've noticed that reload method checks if _language was properly initialized and throws an error otherwise, so I've repeated that solution in instance_has.

My fix will prevent Godot from crashing. However, this does not solve the problem of _language not being initialized. Being a fresh Godot contributor, I'm not sure where it should happen. The value is assigned in init but I don't know whose responsability is it to call init :) Do you think my fix is enough or should I look into it and find a way to initialize _language correctly?

@akien-mga akien-mga added this to the 4.0 milestone May 31, 2021
@akien-mga
Copy link
Member

@pfertyk Your fix seems correct. PluginScript assumes that a call to init should happen first before the rest of the API can be initialized, so using "reverse asserts" like we have with ERR_FAIL_COND is the canonical way to prevent calling such APIs before they're initialized properly.

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

Successfully merging a pull request may close this issue.

4 participants