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.

Sign up for GitHub

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

Jump to bottom

[godot] Memory Corruption in Custom Skins #2489

Closed
jasonwinterpixel opened this issue Mar 23, 2024 · 6 comments
Closed

[godot] Memory Corruption in Custom Skins #2489

jasonwinterpixel opened this issue Mar 23, 2024 · 6 comments
Assignees
@badlogic
Labels
bug

Comments

@jasonwinterpixel
Copy link
Contributor

jasonwinterpixel commented Mar 23, 2024 •
edited

EDIT:
TL;DR
add_skin() is broken
copy_skin() works

The Mix and Match example crashes in the Godot 3.5 editor downloaded from esoteric's website if you reload the scene containing the custom skin. I have an address sanitizer stack trace from a similar game where we load between 2 scenes using custom skins:

=535936==ERROR: AddressSanitizer: heap-use-after-free on address 0x613000071578 at pc 0x000002ecdf2c bp 0x7ffffffd5ab0 sp 0x7ffffffd5aa8
READ of size 4 at 0x613000071578 thread T0
    #0 0x2ecdf2b in spine::Attachment::reference() /home/jason/winterpixel/gooberfall/modules/spine_godot/spine-cpp/src/spine/Attachment.cpp:54:11
    #1 0x204e12a in spine::Skin::addSkin(spine::Skin*) /home/jason/winterpixel/gooberfall/modules/spine_godot/spine-cpp/src/spine/Skin.cpp:167:22
    #2 0x2126bcb in MethodBind1<Ref<SpineSkin> >::call(Object*, Variant const**, int, Variant::CallError&) /home/jason/winterpixel/gooberfall/godot/./core/method_bind.gen.inc:759:3
    #3 0x946ee56 in Object::call(StringName const&, Variant const**, int, Variant::CallError&) /home/jason/winterpixel/gooberfall/godot/core/object.cpp:919:17
    #4 0x966c601 in Variant::call_ptr(StringName const&, Variant const**, int, Variant*, Variant::CallError&) /home/jason/winterpixel/gooberfall/godot/core/variant_call.cpp:1233:14
    #5 0x8b2a47 in GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Variant::CallError&, GDScriptFunction::CallState*) /home/jason/winterpixel/gooberfall/godot/modules/gdscript/gdscript_function.cpp:1056:12
    #6 0x7a24ed in GDScriptInstance::call(StringName const&, Variant const**, int, Variant::CallError&) /home/jason/winterpixel/gooberfall/godot/modules/gdscript/gdscript.cpp:1196:21
    #7 0x946ec5d in Object::call(StringName const&, Variant const**, int, Variant::CallError&) /home/jason/winterpixel/gooberfall/godot/core/object.cpp:900:26
    #8 0x966c601 in Variant::call_ptr(StringName const&, Variant const**, int, Variant*, Variant::CallError&) /home/jason/winterpixel/gooberfall/godot/core/variant_call.cpp:1233:14
    #9 0x8b2a47 in GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Variant::CallError&, GDScriptFunction::CallState*) /home/jason/winterpixel/gooberfall/godot/modules/gdscript/gdscript_function.cpp:1056:12
    #10 0x7a24ed in GDScriptInstance::call(StringName const&, Variant const**, int, Variant::CallError&) /home/jason/winterpixel/gooberfall/godot/modules/gdscript/gdscript.cpp:1196:21
    #11 0x946ec5d in Object::call(StringName const&, Variant const**, int, Variant::CallError&) /home/jason/winterpixel/gooberfall/godot/core/object.cpp:900:26
    #12 0x966c601 in Variant::call_ptr(StringName const&, Variant const**, int, Variant*, Variant::CallError&) /home/jason/winterpixel/gooberfall/godot/core/variant_call.cpp:1233:14
    #13 0x8b2a47 in GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Variant::CallError&, GDScriptFunction::CallState*) /home/jason/winterpixel/gooberfall/godot/modules/gdscript/gdscript_function.cpp:1056:12
    #14 0x7a24ed in GDScriptInstance::call(StringName const&, Variant const**, int, Variant::CallError&) /home/jason/winterpixel/gooberfall/godot/modules/gdscript/gdscript.cpp:1196:21
    #15 0x946ec5d in Object::call(StringName const&, Variant const**, int, Variant::CallError&) /home/jason/winterpixel/gooberfall/godot/core/object.cpp:900:26
    #16 0x966c601 in Variant::call_ptr(StringName const&, Variant const**, int, Variant*, Variant::CallError&) /home/jason/winterpixel/gooberfall/godot/core/variant_call.cpp:1233:14
    #17 0x8b2a47 in GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Variant::CallError&, GDScriptFunction::CallState*) /home/jason/winterpixel/gooberfall/godot/modules/gdscript/gdscript_function.cpp:1056:12
    #18 0x7a2b20 in GDScriptInstance::_ml_call_reversed(GDScript*, StringName const&, Variant const**, int) /home/jason/winterpixel/gooberfall/godot/modules/gdscript/gdscript.cpp:1226:13
    #19 0x63579e0 in Node::_notification(int) /home/jason/winterpixel/gooberfall/godot/scene/main/node.cpp:156:28
    #20 0x94645b6 in Object::notification(int, bool) /home/jason/winterpixel/gooberfall/godot/core/object.cpp:928:2
    #21 0x635c3aa in Node::_propagate_ready() /home/jason/winterpixel/gooberfall/godot/scene/main/node.cpp:189:3
    #22 0x635c2e9 in Node::_propagate_ready() /home/jason/winterpixel/gooberfall/godot/scene/main/node.cpp:181:21
    #23 0x635c2e9 in Node::_propagate_ready() /home/jason/winterpixel/gooberfall/godot/scene/main/node.cpp:181:21
    #24 0x635c2e9 in Node::_propagate_ready() /home/jason/winterpixel/gooberfall/godot/scene/main/node.cpp:181:21
    #25 0x635c2e9 in Node::_propagate_ready() /home/jason/winterpixel/gooberfall/godot/scene/main/node.cpp:181:21
    #26 0x635c2e9 in Node::_propagate_ready() /home/jason/winterpixel/gooberfall/godot/scene/main/node.cpp:181:21
    #27 0x635c2e9 in Node::_propagate_ready() /home/jason/winterpixel/gooberfall/godot/scene/main/node.cpp:181:21
    #28 0x635c2e9 in Node::_propagate_ready() /home/jason/winterpixel/gooberfall/godot/scene/main/node.cpp:181:21
    #29 0x636e5b0 in Node::_set_tree(SceneTree*) /home/jason/winterpixel/gooberfall/godot/scene/main/node.cpp:2809:4
    #30 0x636e5b0 in Node::_add_child_nocheck(Node*, StringName const&) /home/jason/winterpixel/gooberfall/godot/scene/main/node.cpp:1264:12
    #31 0x636ebb9 in Node::add_child(Node*, bool) /home/jason/winterpixel/gooberfall/godot/scene/main/node.cpp:1291:2
    #32 0xe7d225 in MethodBind1<Node*>::call(Object*, Variant const**, int, Variant::CallError&) /home/jason/winterpixel/gooberfall/godot/./core/method_bind.gen.inc:759:3
    #33 0x946ee56 in Object::call(StringName const&, Variant const**, int, Variant::CallError&) /home/jason/winterpixel/gooberfall/godot/core/object.cpp:919:17
    #34 0x944a3da in MessageQueue::_call_function(Object*, StringName const&, Variant const*, int, bool) /home/jason/winterpixel/gooberfall/godot/core/message_queue.cpp:241:12
    #35 0x944ab0f in MessageQueue::flush() /home/jason/winterpixel/gooberfall/godot/core/message_queue.cpp:284:6
    #36 0x6189ae in Main::iteration() /home/jason/winterpixel/gooberfall/godot/main/main.cpp:2311:17
    #37 0x5844c3 in OS_X11::run() /home/jason/winterpixel/gooberfall/godot/platform/x11/os_x11.cpp:3987:7
    #38 0x54bbdb in main /home/jason/winterpixel/gooberfall/godot/platform/x11/godot_x11.cpp:60:6
    #39 0x7ffff7aa0d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #40 0x7ffff7aa0e3f in __libc_start_main csu/../csu/libc-start.c:392:3
    #41 0x48b7c4 in _start (/home/jason/winterpixel/gooberfall/godot/bin/godot.x11.opt.tools.64.llvms+0x48b7c4) (BuildId: 18b434e684ccda645ff6cf2ba2f979bcce046748)

0x613000071578 is located 56 bytes inside of 376-byte region [0x613000071540,0x6130000716b8)
freed by thread T0 here:
    #0 0x50e362 in __interceptor_free (/home/jason/winterpixel/gooberfall/godot/bin/godot.x11.opt.tools.64.llvms+0x50e362) (BuildId: 18b434e684ccda645ff6cf2ba2f979bcce046748)
    #1 0x2053b46 in void spine::SpineExtension::free<void>(void*, char const*, int) /home/jason/winterpixel/gooberfall/modules/spine_godot/spine-cpp/include/spine/Extension.h:61:19
    #2 0x2053b46 in spine::SpineObject::operator delete(void*) /home/jason/winterpixel/gooberfall/modules/spine_godot/spine-cpp/src/spine/SpineObject.cpp:58:2
    #3 0x204cdf8 in spine::Skin::~Skin() /home/jason/winterpixel/gooberfall/modules/spine_godot/spine-cpp/src/spine/Skin.cpp:95:15
    #4 0x9492666 in Object::notification(int, bool) /home/jason/winterpixel/gooberfall/godot/core/object.cpp:928:2
    #5 0x9492666 in Object::_predelete() /home/jason/winterpixel/gooberfall/godot/core/object.cpp:388:2
    #6 0x9492666 in predelete_handler(Object*) /home/jason/winterpixel/gooberfall/godot/core/object.cpp:2013:19
    #7 0x9492666 in Object::notification(int, bool) /home/jason/winterpixel/gooberfall/godot/core/object.cpp:928:2
    #8 0x9492666 in Object::_predelete() /home/jason/winterpixel/gooberfall/godot/core/object.cpp:388:2
    #9 0x9492666 in predelete_handler(Object*) /home/jason/winterpixel/gooberfall/godot/core/object.cpp:2013:19
    #10 0x9492666 in Object::notification(int, bool) /home/jason/winterpixel/gooberfall/godot/core/object.cpp:928:2
    #11 0x9492666 in Object::_predelete() /home/jason/winterpixel/gooberfall/godot/core/object.cpp:388:2
    #12 0x9492666 in predelete_handler(Object*) /home/jason/winterpixel/gooberfall/godot/core/object.cpp:2013:19
    #13 0x9492666 in Object::notification(int, bool) /home/jason/winterpixel/gooberfall/godot/core/object.cpp:928:2
    #14 0x9492666 in Object::_predelete() /home/jason/winterpixel/gooberfall/godot/core/object.cpp:388:2
    #15 0x9492666 in predelete_handler(Object*) /home/jason/winterpixel/gooberfall/godot/core/object.cpp:2013:19
    #16 0x9492666 in Object::notification(int, bool) /home/jason/winterpixel/gooberfall/godot/core/object.cpp:928:2
    #17 0x9492666 in Object::_predelete() /home/jason/winterpixel/gooberfall/godot/core/object.cpp:388:2
    #18 0x9492666 in predelete_handler(Object*) /home/jason/winterpixel/gooberfall/godot/core/object.cpp:2013:19
    #19 0x946ee56 in Object::call(StringName const&, Variant const**, int, Variant::CallError&) /home/jason/winterpixel/gooberfall/godot/core/object.cpp:919:17
    #20 0x944a3da in MessageQueue::_call_function(Object*, StringName const&, Variant const*, int, bool) /home/jason/winterpixel/gooberfall/godot/core/message_queue.cpp:241:12
    #21 0x944ab0f in MessageQueue::flush() /home/jason/winterpixel/gooberfall/godot/core/message_queue.cpp:284:6

previously allocated by thread T0 here:
    #0 0x50e60e in malloc (/home/jason/winterpixel/gooberfall/godot/bin/godot.x11.opt.tools.64.llvms+0x50e60e) (BuildId: 18b434e684ccda645ff6cf2ba2f979bcce046748)
    #1 0x988a730 in Memory::alloc_static(unsigned long, bool) /home/jason/winterpixel/gooberfall/godot/core/os/memory.cpp:84:14
    #2 0x2f016dd in spine::SkeletonBinary::readSkin(spine::SkeletonBinary::DataInput*, bool, spine::SkeletonData*, bool) /home/jason/winterpixel/gooberfall/modules/spine_godot/spine-cpp/src/spine/SkeletonBinary.cpp:486:29
    #3 0x2ef97b0 in spine::SkeletonBinary::readSkeletonData(unsigned char const*, int) /home/jason/winterpixel/gooberfall/modules/spine_godot/spine-cpp/src/spine/SkeletonBinary.cpp:279:16
    #4 0x21471cf in SpineSkeletonDataResource::load_resources(spine::Atlas*, String const&, Vector<unsigned char> const&) /home/jason/winterpixel/gooberfall/modules/spine_godot/SpineSkeletonDataResource.cpp:176:25
    #5 0x2145d8b in SpineSkeletonDataResource::update_skeleton_data() /home/jason/winterpixel/gooberfall/modules/spine_godot/SpineSkeletonDataResource.cpp:154:3
    #6 0x2140b20 in SpineSkeletonDataResource::set_skeleton_file_res(Ref<SpineSkeletonFileResource> const&) /home/jason/winterpixel/gooberfall/modules/spine_godot/SpineSkeletonDataResource.cpp:221:2
    #7 0x2156f14 in MethodBind1<Ref<SpineSkeletonFileResource> const&>::call(Object*, Variant const**, int, Variant::CallError&) /home/jason/winterpixel/gooberfall/godot/./core/method_bind.gen.inc:759:3
    #8 0x92fa7ad in ClassDB::set_property(Object*, StringName const&, Variant const&, bool*) /home/jason/winterpixel/gooberfall/godot/core/class_db.cpp:1015:20
    #9 0x9464956 in Object::set(StringName const&, Variant const&, bool*) /home/jason/winterpixel/gooberfall/godot/core/object.cpp:423:7
    #10 0x7c79b4a in ResourceInteractiveLoaderText::poll() /home/jason/winterpixel/gooberfall/godot/scene/resources/resource_format_text.cpp:573:15
    #11 0x9b99efe in ResourceFormatLoader::load(String const&, String const&, Error*) /home/jason/winterpixel/gooberfall/godot/core/io/resource_loader.cpp:193:20
    #12 0x9b9f41f in ResourceLoader::_load(String const&, String const&, String const&, bool, Error*) /home/jason/winterpixel/gooberfall/godot/core/io/resource_loader.cpp:262:24
    #13 0x9ba1619 in ResourceLoader::load(String const&, String const&, bool, Error*) /home/jason/winterpixel/gooberfall/godot/core/io/resource_loader.cpp:376:12
    #14 0x7c7ac80 in ResourceInteractiveLoaderText::poll() /home/jason/winterpixel/gooberfall/godot/scene/resources/resource_format_text.cpp:406:13
    #15 0x9b99efe in ResourceFormatLoader::load(String const&, String const&, Error*) /home/jason/winterpixel/gooberfall/godot/core/io/resource_loader.cpp:193:20
    #16 0x9b9f41f in ResourceLoader::_load(String const&, String const&, String const&, bool, Error*) /home/jason/winterpixel/gooberfall/godot/core/io/resource_loader.cpp:262:24
    #17 0x9ba1619 in ResourceLoader::load(String const&, String const&, bool, Error*) /home/jason/winterpixel/gooberfall/godot/core/io/resource_loader.cpp:376:12
    #18 0x7c7ac80 in ResourceInteractiveLoaderText::poll() /home/jason/winterpixel/gooberfall/godot/scene/resources/resource_format_text.cpp:406:13
    #19 0xa9f17e in MethodBind0R<Error>::call(Object*, Variant const**, int, Variant::CallError&) /home/jason/winterpixel/gooberfall/godot/./core/method_bind.gen.inc:233:17
    #20 0x946ee56 in Object::call(StringName const&, Variant const**, int, Variant::CallError&) /home/jason/winterpixel/gooberfall/godot/core/object.cpp:919:17
    #21 0x966c601 in Variant::call_ptr(StringName const&, Variant const**, int, Variant*, Variant::CallError&) /home/jason/winterpixel/gooberfall/godot/core/variant_call.cpp:1233:14
    #22 0x8c9ec2 in GDScriptFunction::call(GDScriptInstance*, Variant const**, int, Variant::CallError&, GDScriptFunction::CallState*) /home/jason/winterpixel/gooberfall/godot/modules/gdscript/gdscript_function.cpp:1054:12
    #23 0x7a27ae in GDScriptInstance::call_multilevel(StringName const&, Variant const**, int) /home/jason/winterpixel/gooberfall/godot/modules/gdscript/gdscript.cpp:1211:14
    #24 0x6357b75 in Node::_notification(int) /home/jason/winterpixel/gooberfall/godot/scene/main/node.cpp:58:28
    #25 0x94645b6 in Object::notification(int, bool) /home/jason/winterpixel/gooberfall/godot/core/object.cpp:928:2
@jasonwinterpixel
Copy link
Contributor Author

jasonwinterpixel commented Mar 23, 2024 •
edited

To reproduce: add a button with this script on it to the mix and match example scene and press the button:

extends BaseButton

func _pressed():
	get_tree().reload_current_scene()

@jasonwinterpixel
Copy link
Contributor Author

same bug i think:
#2202

@jasonwinterpixel
Copy link
Contributor Author

same bug: #2185

@jasonwinterpixel
Copy link
Contributor Author

Also crashes godot 4.1, ie: this https://spine-godot.s3.eu-central-1.amazonaws.com/4.1/4.1.3-stable/godot-editor-linux.zip

@jasonwinterpixel
Copy link
Contributor Author

jasonwinterpixel commented Mar 23, 2024 •
edited

Note: using 'copy skin' instead of 'add skin' doesnt crash

What is the difference? Internal code looks almost identical except for the way mesh attachments are handled.

@jasonwinterpixel jasonwinterpixel changed the title [godot] Memory Corruption in Custom Skins in Godot 3.5 [godot] Memory Corruption in Custom Skins Mar 23, 2024
@badlogic badlogic self-assigned this Mar 28, 2024
@badlogic badlogic added the bug label Mar 28, 2024
@badlogic
Copy link
Collaborator

Sorry for this taking so long. Attachments use a reference counting mechanism so they an be shared between skin instances. There was a bug with respect to the handling of this system (part of spine-cpp) in spine-godot. This was quite hard to track down.

This should be fixed in the 4.2 branch now. Thanks for reporting.

badlogic added a commit that referenced this issue May 21, 2024
@badlogic
[godot] Closes #2489, fix reference count of SpineAttachments
5abed8a 
The SpineAttachment destructor would dereference the spine::Attachment. However, the set_spine_object() call doesn't increase the reference count. This can lead to the attachment being deallocated too early.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@badlogic badlogic

Labels
bug
Milestone
No milestone
Development

No branches or pull requests
2 participants
@badlogic @jasonwinterpixel