search

godotengine / godot

Godot Engine – Multi-platform 2D and 3D game engine
https://godotengine.org
MIT License
90.21k stars 21.04k forks source link

Issues with passing larger godot::Strings around multiple DLLs #49697

Open rbaileye opened 3 years ago

rbaileye commented 3 years ago

Godot version

3.3.2 Stable

System information

Windows 10 version 10.0.19041

Issue description

To start off with I might just be doing this in incorrect way, if so just let me know.

Issue: The passing of Strings once they get big causes crashes when passing them across Dll's using GDNativeScript. I noticed this problem within my own project, once the Strings start to get to the size of around 100000 characters. Sometimes it would work sometimes it wouldn't. The client was closing without errors or any reporting.

I then created a separate project for testing strings of a larger size around 5000000 characters in a separate project I started getting crash dumps listed below.

Dump from crash

4999998
CrashHandlerException: Program crashed
ERROR: NativeScriptInstance::notification: NativeScriptInstance detected crash on method: test_big_string
   At: modules\gdnative\nativescript\nativescript.cpp:736
ERROR: NativeScriptInstance::notification: NativeScriptInstance detected crash on method: pass_method2
   At: modules\gdnative\nativescript\nativescript.cpp:736
Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[0] CowData<char>::size (C:\git\godot-3.3-stable\godot-3.3-stable\core\cowdata.h:132)
[1] CharString::size (C:\git\godot-3.3-stable\godot-3.3-stable\core\ustring.h:81)
[2] CharString::get_data (C:\git\godot-3.3-stable\godot-3.3-stable\core\ustring.cpp:123)
[3] godot_char_string_get_data (C:\git\godot-3.3-stable\godot-3.3-stable\modules\gdnative\gdnative\string.cpp:57)
[4] godot::CharString::get_data (c:\git\TestBuildHeaders\godot-cpp-3.3\src\core\String.cpp:25)
[5] TestClass2::pass_method2 (C:\git\TestBuildHeaders\gdnative-demos-master\cpp\bindings\Exampledlls\Dll2\Dll2\TestClass2.hpp:89)
[6] godot::_WrappedMethod<TestClass2,void,godot::String const &>::apply<0> (C:\git\TestBuildHeaders\gdnative-demos-master\cpp\bindings\core\Godot.hpp:264)
[7] godot::__wrapped_method<TestClass2,void,godot::String const &> (C:\git\TestBuildHeaders\gdnative-demos-master\cpp\bindings\core\Godot.hpp:280)
[8] NativeScriptInstance::call (C:\git\godot-3.3-stable\godot-3.3-stable\modules\gdnative\nativescript\nativescript.cpp:709)
[9] Object::call (C:\git\godot-3.3-stable\godot-3.3-stable\core\object.cpp:898)
[10] Variant::call_ptr (C:\git\godot-3.3-stable\godot-3.3-stable\core\variant_call.cpp:1149)
[11] GDScriptFunction::call (C:\git\godot-3.3-stable\godot-3.3-stable\modules\gdscript\gdscript_function.cpp:1089)
[12] GDScriptInstance::call (C:\git\godot-3.3-stable\godot-3.3-stable\modules\gdscript\gdscript.cpp:1208)
[13] Object::call (C:\git\godot-3.3-stable\godot-3.3-stable\core\object.cpp:898)
[14] Object::emit_signal (C:\git\godot-3.3-stable\godot-3.3-stable\core\object.cpp:1246)
[15] Object::_emit_signal (C:\git\godot-3.3-stable\godot-3.3-stable\core\object.cpp:1179)
[16] MethodBindVarArg<Object>::call (C:\git\godot-3.3-stable\godot-3.3-stable\core\method_bind.h:344)
[17] godot_method_bind_call (C:\git\godot-3.3-stable\godot-3.3-stable\modules\gdnative\gdnative\gdnative.cpp:83)
[18] godot::Object::emit_signal (c:\git\TestBuildHeaders\godot-cpp-3.3\src\gen\Object.cpp:183)
[19] godot::Object::emit_signal<godot::String> (C:\git\TestBuildHeaders\gdnative-demos-master\cpp\bindings\gen\Object.hpp:155)
[20] TestClass::test_big_string (C:\git\TestBuildHeaders\gdnative-demos-master\cpp\bindings\Exampledlls\Dll1\Dll1\TestClass.hpp:39)
[21] godot::_WrappedMethod<TestClass,void>::apply<> (C:\git\TestBuildHeaders\gdnative-demos-master\cpp\bindings\core\Godot.hpp:264)
[22] godot::__wrapped_method<TestClass,void> (C:\git\TestBuildHeaders\gdnative-demos-master\cpp\bindings\core\Godot.hpp:280)
[23] NativeScriptInstance::call (C:\git\godot-3.3-stable\godot-3.3-stable\modules\gdnative\nativescript\nativescript.cpp:709)
[24] Object::call (C:\git\godot-3.3-stable\godot-3.3-stable\core\object.cpp:898)
[25] Variant::call_ptr (C:\git\godot-3.3-stable\godot-3.3-stable\core\variant_call.cpp:1149)
[26] GDScriptFunction::call (C:\git\godot-3.3-stable\godot-3.3-stable\modules\gdscript\gdscript_function.cpp:1089)
[27] GDScriptInstance::_ml_call_reversed (C:\git\godot-3.3-stable\godot-3.3-stable\modules\gdscript\gdscript.cpp:1239)
[28] GDScriptInstance::call_multilevel_reversed (C:\git\godot-3.3-stable\godot-3.3-stable\modules\gdscript\gdscript.cpp:1248)
[29] Node::_notification (C:\git\godot-3.3-stable\godot-3.3-stable\scene\main\node.cpp:152)
[30] Node::_notificationv (C:\git\godot-3.3-stable\godot-3.3-stable\scene\main\node.h:46)
[31] CanvasItem::_notificationv (C:\git\godot-3.3-stable\godot-3.3-stable\scene\2d\canvas_item.h:166)
[32] Control::_notificationv (C:\git\godot-3.3-stable\godot-3.3-stable\scene\gui\control.h:48)
[33] Object::notification (C:\git\godot-3.3-stable\godot-3.3-stable\core\object.cpp:931)
[34] Node::_propagate_ready (C:\git\godot-3.3-stable\godot-3.3-stable\scene\main\node.cpp:197)
[35] Node::_propagate_ready (C:\git\godot-3.3-stable\godot-3.3-stable\scene\main\node.cpp:189)
[36] Node::_set_tree (C:\git\godot-3.3-stable\godot-3.3-stable\scene\main\node.cpp:2626)
[37] SceneTree::init (C:\git\godot-3.3-stable\godot-3.3-stable\scene\main\scene_tree.cpp:466)
[38] OS_Windows::run (C:\git\godot-3.3-stable\godot-3.3-stable\platform\windows\os_windows.cpp:3437)
[39] widechar_main (C:\git\godot-3.3-stable\godot-3.3-stable\platform\windows\godot_windows.cpp:162)
[40] _main (C:\git\godot-3.3-stable\godot-3.3-stable\platform\windows\godot_windows.cpp:184)
[41] main (C:\git\godot-3.3-stable\godot-3.3-stable\platform\windows\godot_windows.cpp:196)
[42] __scrt_common_main_seh (D:\agent\_work\13\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
[43] BaseThreadInitThunk
-- END OF BACKTRACE --

Steps to reproduce

  1. Take a String of size 5000000 with whatever ascii data in it, I just filled it with the letter A in a (lets call it) Dll1.

  2. Pass that data to a Godot engine class using signals.

  3. Assign that value to a class variable, this appears to be fine and will print, give you size, etc.

  4. Take that assigned variable and pass it to another (lets call it) Dll2. You can try either passing it directly through a method call or a signal.

  5. Now try to get String.ascii.get_data(), or const char* data to do some manipulation on it. This is where it will fail and crash dump. (Note)This normally can produce the length properly. Using String.length() function and printing it. Right before it fails.

Minimal reproduction project

I have uploaded a build project for a minimal setup with some notes in it on building and such.

https://github.com/rbaileye/GDNativeStringIssue

Thanks for your time.

rbaileye commented 3 years ago

Just in case there down the road someone else comes across this issue and the bug isn't fixed, or you want more performance. As I have found out creating large Strings and using them with the engine and DLL's isn't the perfect or optimal at times.

I have figured out a workout to this issue, by creating another variant type(void) in the Engine and Gdnative module. And the ability to create that variant to pass around Dll's. With this I am not copying the data, and just passing that variant type around and just the pointer as expected on the other side. This isn't a solution to the known bug, so I won't post those changes I did here.