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

GDScript LSP parse_local_script() crashes the editor #39375

Closed
Zylann opened this issue Jun 7, 2020 · 7 comments · Fixed by #39385
Closed

GDScript LSP parse_local_script() crashes the editor #39375

Zylann opened this issue Jun 7, 2020 · 7 comments · Fixed by #39385

Comments

@Zylann
Copy link
Contributor

Zylann commented Jun 7, 2020

Godot 3.2.2 beta4

After reading how https://github.com/GDQuest/gdscript-docs-maker/blob/4c3020988c2542737abf467a7ae40d9f744e544c/godot-scripts/Collector.gd#L74 was written, I discovered there is an apparently undocumented access to GDScript language server, which I'd find very useful to have information about.

So I tried to grab that data and see what it provides, using a similar script. I used an EditorPlugin because that singleton doesn't exist in non-editor launches. Unfortunately, it crashes the editor when my script gets to call workspace.parse_local_script(file).

var data := {
	name = ProjectSettings.get_setting("application/config/name"),
	description = ProjectSettings.get_setting("application/config/description"),
	version = ProjectSettings.get_setting("application/config/version"),
	classes = []
}
var workspace = Engine.get_singleton('GDScriptLanguageProtocol').get_workspace()
for file in ["addons/zylann.gdscript_doc/gdscript_extractor.gd"]:
	workspace.parse_local_script(file) # CRASHES
	var symbols: Dictionary = workspace.generate_script_api(file)
	data["classes"].append(symbols)
var json = JSON.print(data, "   ")
var f = File.new()
f.open("result.json", File.WRITE)
f.store_string(json)
f.close()
ERROR: get: Condition "!res" is true. Returned: *res
   At: ./core/hash_map.h:304

Test project:
GDScriptDocCrash.zip
Go to ProjectSettings and enable the plugin.

@akien-mga
Copy link
Member

Stacktrace from the master branch:

[1] /lib64/libc.so.6(+0x3a3f0) [0x7f11528e73f0] (??:0)
[2] Ref<GDScriptLanguageProtocol::LSPeer>::ref(Ref<GDScriptLanguageProtocol::LSPeer> const&) (/home/akien/Projects/godot/godot.git/./core/reference.h:62)
[3] Ref<GDScriptLanguageProtocol::LSPeer>::Ref(Ref<GDScriptLanguageProtocol::LSPeer> const&) (/home/akien/Projects/godot/godot.git/./core/reference.h:179)
[4] GDScriptLanguageProtocol::notify_client(String const&, Variant const&, int) (/home/akien/Projects/godot/godot.git/modules/gdscript/language_server/gdscript_language_protocol.cpp:274)
[5] GDScriptWorkspace::publish_diagnostics(String const&) (/home/akien/Projects/godot/godot.git/modules/gdscript/language_server/gdscript_workspace.cpp:374 (discriminator 5))
[6] GDScriptWorkspace::parse_script(String const&, String const&) (/home/akien/Projects/godot/godot.git/modules/gdscript/language_server/gdscript_workspace.cpp:336)
[7] GDScriptWorkspace::parse_local_script(String const&) (/home/akien/Projects/godot/godot.git/modules/gdscript/language_server/gdscript_workspace.cpp:343)
[8] MethodBind1R<Error, String const&>::call(Object*, Variant const**, int, Callable::CallError&) (/home/akien/Projects/godot/godot.git/./core/method_bind.gen.inc:961 (discriminator 8))
[9] Object::call(StringName const&, Variant const**, int, Callable::CallError&) (/home/akien/Projects/godot/godot.git/core/object.cpp:890 (discriminator 1))
[10] Variant::call_ptr(StringName const&, Variant const**, int, Variant*, Callable::CallError&) (/home/akien/Projects/godot/godot.git/core/variant_call.cpp:1226 (discriminator 1))
[11] GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Callable::CallError&, GDScriptFunction::CallState*) (/home/akien/Projects/godot/godot.git/modules/gdscript/gdscript_function.cpp:1047)
[12] GDScriptInstance::call(StringName const&, Variant const**, int, Callable::CallError&) (/home/akien/Projects/godot/godot.git/modules/gdscript/gdscript.cpp:1291)
[13] Object::call(StringName const&, Variant const**, int, Callable::CallError&) (/home/akien/Projects/godot/godot.git/core/object.cpp:871 (discriminator 1))
[14] Variant::call_ptr(StringName const&, Variant const**, int, Variant*, Callable::CallError&) (/home/akien/Projects/godot/godot.git/core/variant_call.cpp:1226 (discriminator 1))
[15] GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Callable::CallError&, GDScriptFunction::CallState*) (/home/akien/Projects/godot/godot.git/modules/gdscript/gdscript_function.cpp:1047)
[16] GDScriptInstance::_ml_call_reversed(GDScript*, StringName const&, Variant const**, int) (/home/akien/Projects/godot/godot.git/modules/gdscript/gdscript.cpp:1321)
[17] GDScriptInstance::call_multilevel_reversed(StringName const&, Variant const**, int) (/home/akien/Projects/godot/godot.git/modules/gdscript/gdscript.cpp:1329)
[18] Node::_propagate_enter_tree() (/home/akien/Projects/godot/godot.git/scene/main/node.cpp:222)
[19] Node::_set_tree(SceneTree*) (/home/akien/Projects/godot/godot.git/scene/main/node.cpp:2552)
[20] Node::_add_child_nocheck(Node*, StringName const&) (/home/akien/Projects/godot/godot.git/scene/main/node.cpp:1221)
[21] Node::add_child(Node*, bool) (/home/akien/Projects/godot/godot.git/scene/main/node.cpp:1235)
[22] EditorNode::add_editor_plugin(EditorPlugin*, bool) (/home/akien/Projects/godot/godot.git/editor/editor_node.cpp:2798)
[23] EditorNode::set_addon_plugin_enabled(String const&, bool, bool) (/home/akien/Projects/godot/godot.git/editor/editor_node.cpp:2928)
[24] EditorPluginSettings::_plugin_activity_changed() (/home/akien/Projects/godot/godot.git/editor/editor_plugin_settings.cpp:160)
[25] void call_with_variant_args_helper<EditorPluginSettings>(EditorPluginSettings*, void (EditorPluginSettings::*)(), Variant const**, Callable::CallError&, IndexSequence<>) (/home/akien/Projects/godot/godot.git/./core/callable_method_pointer.h:136 (discriminator 4))
[26] void call_with_variant_args<EditorPluginSettings>(EditorPluginSettings*, void (EditorPluginSettings::*)(), Variant const**, int, Callable::CallError&) (/home/akien/Projects/godot/godot.git/./core/callable_method_pointer.h:158)
[27] CallableCustomMethodPointer<EditorPluginSettings>::call(Variant const**, int, Variant&, Callable::CallError&) const (/home/akien/Projects/godot/godot.git/./core/callable_method_pointer.h:185)
[28] Callable::call(Variant const**, int, Variant&, Callable::CallError&) const (/home/akien/Projects/godot/godot.git/core/callable.cpp:54)
[29] Object::emit_signal(StringName const&, Variant const**, int) (/home/akien/Projects/godot/godot.git/core/object.cpp:1170)
[30] Object::emit_signal(StringName const&, Variant const&, Variant const&, Variant const&, Variant const&, Variant const&) (/home/akien/Projects/godot/godot.git/core/object.cpp:1225)
[31] Tree::item_edited(int, TreeItem*, bool) (/home/akien/Projects/godot/godot.git/scene/gui/tree.cpp:3013 (discriminator 1))
[32] Tree::propagate_mouse_event(Vector2i const&, int, int, bool, TreeItem*, int, Ref<InputEventWithModifiers> const&) (/home/akien/Projects/godot/godot.git/scene/gui/tree.cpp:1805)
[33] Tree::propagate_mouse_event(Vector2i const&, int, int, bool, TreeItem*, int, Ref<InputEventWithModifiers> const&) (/home/akien/Projects/godot/godot.git/scene/gui/tree.cpp:1922)
[34] Tree::_gui_input(Ref<InputEvent>) (/home/akien/Projects/godot/godot.git/scene/gui/tree.cpp:2565 (discriminator 5))
[35] MethodBind1<Ref<InputEvent> >::call(Object*, Variant const**, int, Callable::CallError&) (/home/akien/Projects/godot/godot.git/./core/method_bind.gen.inc:775 (discriminator 12))
[36] Object::call_multilevel(StringName const&, Variant const**, int) (/home/akien/Projects/godot/godot.git/core/object.cpp:737 (discriminator 1))
[37] Object::call_multilevel(StringName const&, Variant const&, Variant const&, Variant const&, Variant const&, Variant const&) (/home/akien/Projects/godot/godot.git/core/object.cpp:836)
[38] Viewport::_gui_call_input(Control*, Ref<InputEvent> const&) (/home/akien/Projects/godot/godot.git/scene/main/viewport.cpp:1610 (discriminator 1))
[39] Viewport::_gui_input_event(Ref<InputEvent>) (/home/akien/Projects/godot/godot.git/scene/main/viewport.cpp:1867 (discriminator 3))
[40] Viewport::input(Ref<InputEvent> const&, bool) (/home/akien/Projects/godot/godot.git/scene/main/viewport.cpp:2921 (discriminator 3))
[41] Window::_window_input(Ref<InputEvent> const&) (/home/akien/Projects/godot/godot.git/scene/main/window.cpp:886 (discriminator 1))
[42] Viewport::_sub_windows_forward_input(Ref<InputEvent> const&) (/home/akien/Projects/godot/godot.git/scene/main/viewport.cpp:2888)
[43] Viewport::input(Ref<InputEvent> const&, bool) (/home/akien/Projects/godot/godot.git/scene/main/viewport.cpp:2911 (discriminator 1))
[44] Window::_window_input(Ref<InputEvent> const&) (/home/akien/Projects/godot/godot.git/scene/main/window.cpp:886 (discriminator 1))
[45] void call_with_variant_args_helper<Window, Ref<InputEvent> const&, 0ul>(Window*, void (Window::*)(Ref<InputEvent> const&), Variant const**, Callable::CallError&, IndexSequence<0ul>) (/home/akien/Projects/godot/godot.git/./core/callable_method_pointer.h:132 (discriminator 6))
[46] void call_with_variant_args<Window, Ref<InputEvent> const&>(Window*, void (Window::*)(Ref<InputEvent> const&), Variant const**, int, Callable::CallError&) (/home/akien/Projects/godot/godot.git/./core/callable_method_pointer.h:158)
[47] CallableCustomMethodPointer<Window, Ref<InputEvent> const&>::call(Variant const**, int, Variant&, Callable::CallError&) const (/home/akien/Projects/godot/godot.git/./core/callable_method_pointer.h:185)
[48] Callable::call(Variant const**, int, Variant&, Callable::CallError&) const (/home/akien/Projects/godot/godot.git/core/callable.cpp:54)
[49] DisplayServerX11::_dispatch_input_event(Ref<InputEvent> const&) (/home/akien/Projects/godot/godot.git/platform/linuxbsd/display_server_x11.cpp:2302)
[50] DisplayServerX11::_dispatch_input_events(Ref<InputEvent> const&) (/home/akien/Projects/godot/godot.git/platform/linuxbsd/display_server_x11.cpp:2286)
[51] Input::_parse_input_event_impl(Ref<InputEvent> const&, bool) (/home/akien/Projects/godot/godot.git/core/input/input.cpp:590)
[52] Input::parse_input_event(Ref<InputEvent> const&) (/home/akien/Projects/godot/godot.git/core/input/input.cpp:440)
[53] Input::flush_accumulated_events() (/home/akien/Projects/godot/godot.git/core/input/input.cpp:830)
[54] DisplayServerX11::process_events() (/home/akien/Projects/godot/godot.git/./core/os/mutex.h:70)
[55] OS_LinuxBSD::run() (/home/akien/Projects/godot/godot.git/platform/linuxbsd/os_linuxbsd.cpp:239)
[56] godot-git(main+0xfa) [0x19e39ac] (/home/akien/Projects/godot/godot.git/platform/linuxbsd/godot_linuxbsd.cpp:57)
[57] /lib64/libc.so.6(__libc_start_main+0xea) [0x7f11528d3cda] (??:0)
[58] godot-git(_start+0x2a) [0x19e380a] (??:?)

Had to tweak the data dictionary so that it compiles in the master branch, the name = value Dictionary syntax is broken right now - likely due to StringName changes:

var data := {
	"name": ProjectSettings.get_setting("application/config/name"),
	"description": ProjectSettings.get_setting("application/config/description"),
	"version": ProjectSettings.get_setting("application/config/version"),
	"classes": []
}

@akien-mga
Copy link
Member

The original MRP is a bit evil as it's a tool script that tries to parse itself... but the same crash happens when parsing another script without circular reference.

akien-mga added a commit to akien-mga/godot that referenced this issue Jun 8, 2020
`latest_client_id` now defaults to `-1` (invalid ID) instead of `0`.

Also fix typo in notification `gdscrip_client/changeWorkspace`,
and fix argument names in method binds.

Fixes godotengine#39375.
@Zylann
Copy link
Contributor Author

Zylann commented Jun 8, 2020

Will this be fixed in 3.2? Also is this a misuse? (there is no doc for it) The GDScript doc maker uses it, and I would really like to use that data source as well...

The original MRP is a bit evil as it's a tool script that tries to parse itself...

I don't see how that's a problem, it shouldnt be. It's not like I'm doing preload here :p

@akien-mga
Copy link
Member

akien-mga commented Jun 8, 2020

Will this be fixed in 3.2?

Yes, that's what this label means:
Screenshot_20200608_132816

Also is this a misuse?

I think so, but I'm not familiar enough with this to confirm 100%. Reading the code, it seems latest_client_id (which was invalid and triggering a crash in your code) is only assigned in GDScriptLanguageProtocol::poll(), so I guess you need to poll() - or this might be done automatically when a client is connected, so I guess what you might need is to connect a client.

The API should definitely be documented and improved further though.

@Zylann
Copy link
Contributor Author

Zylann commented Jun 8, 2020

I can't find any place in this repo where poll is called: https://github.com/GDQuest/gdscript-docs-maker/search?q=poll&unscoped_q=poll
It just calls parse_local_script directly, and the script is called from a CLI execution of Godot.

Note I'm just interested to get the GDScript parsing data from within the Godot editor, not even trying to write a client/server thing. I've been looking for a way to get that data for a while, but so far it seems to remain behind an API meant for something else...

@akien-mga
Copy link
Member

Maybe @NathanLovato and @Razoric480 (who authored the GDQuest script) can weigh in.

Maybe parse_local_script could be made not to call notify_clients if there are no clients to notify. After my fix, it will currently raise an error when notify_clients is called without clients, but parse_local_script itself should succeed.

@Razoric480
Copy link
Contributor

I can't really comment on this issue, despite me and Nathan working on the script that uses it; we never really encountered an issue with it, maybe because we specifically had used it in a way that it wouldn't reference itself (having the scripts in the root of the project, pointing to a specific directory) or because we run it from inside a launched editor instance.

Either way, it was sheer happenstance rather than a deliberate bug avoidance.

akien-mga added a commit to akien-mga/godot that referenced this issue Jun 10, 2020
`latest_client_id` now defaults to `-1` (invalid ID) instead of `0`.

Also fix typo in notification `gdscrip_client/changeWorkspace`,
and fix argument names in method binds.

Fixes godotengine#39375.

(cherry picked from commit e34f337)
ChristopheLY pushed a commit to ChristopheLY/godot that referenced this issue Jun 22, 2020
`latest_client_id` now defaults to `-1` (invalid ID) instead of `0`.

Also fix typo in notification `gdscrip_client/changeWorkspace`,
and fix argument names in method binds.

Fixes godotengine#39375.
huhund pushed a commit to huhund/godot that referenced this issue Nov 10, 2020
`latest_client_id` now defaults to `-1` (invalid ID) instead of `0`.

Also fix typo in notification `gdscrip_client/changeWorkspace`,
and fix argument names in method binds.

Fixes godotengine#39375.

(cherry picked from commit e34f337)
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