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

Editor Crashed using GraphEdit with instanced GraphNode #45072

Closed
ghost opened this issue Jan 10, 2021 · 9 comments · Fixed by #45149
Closed

Editor Crashed using GraphEdit with instanced GraphNode #45072

ghost opened this issue Jan 10, 2021 · 9 comments · Fixed by #45149

Comments

@ghost
Copy link

ghost commented Jan 10, 2021

Godot version:
Godot v3.2.4 beta 5

OS/device including version:
EndeavourOS Linux

Issue description:
Editor crashed when working with GraphEdit and instanced GraphNode. Saving a scene with GraphNode as a base node, and return to the scene where it was instanced, make the editor crashed.

// Representation of sceen tree that I tested

// Not crashed
Control
    GraphEdit // Not crashed even if not inside tree
    GraphNode
    GraphNode(Instanced)

// Crashed
Control
    GraphEdit
        GraphNode(Instanced)
    GraphNode

// Crashed
Control
    GraphEdit
        GraphNode
    GraphNode(Instanced)
Backtrace

handle_crash: Program crashed with signal 11
Dumping the backtrace. Please include this when reporting the bug
[1] /usr/lib/libc.so.6(+0x3d6a0) [0x7faa7135a6a0] (??:0)
[2] /home/nafys/dev/godot/godot3() [0xda8e4e] (??:?)
[3] /home/nafys/dev/godot/godot3() [0x1b7704a] (:?)
[4] /home/nafys/dev/godot/godot3() [0x1c822aa] (??:?)
[5] /home/nafys/dev/godot/godot3() [0x1c9e2d2] (??:?)
[6] /home/nafys/dev/godot/godot3() [0xdbc65b] (??:?)
[7] /home/nafys/dev/godot/godot3() [0x2cef619] (??:?)
[8] /home/nafys/dev/godot/godot3() [0x1c9e2da] (??:?)
[9] /home/nafys/dev/godot/godot3() [0xdbc65b] (??:?)
[10] /home/nafys/dev/godot/godot3() [0x2cef619] (??:?)
[11] /home/nafys/dev/godot/godot3() [0x25375ef] (??:?)
[12] /home/nafys/dev/godot/godot3() [0x2e58934] (??:?)
[13] /home/nafys/dev/godot/godot3() [0xde38f6] (??:?)
[14] /home/nafys/dev/godot/godot3() [0xd6a483] (:?)
[15] /home/nafys/dev/godot/godot3() [0xd6d339] (??:?)
[16] /home/nafys/dev/godot/godot3() [0xd6d570] (??:?)
[17] /home/nafys/dev/godot/godot3() [0x24bc0b7] (??:?)
[18] /home/nafys/dev/godot/godot3() [0x2e58934] (??:?)
[19] /home/nafys/dev/godot/godot3() [0xde38f6] (??:?)
[20] /home/nafys/dev/godot/godot3() [0xdd36e4] (??:?)
[21] /home/nafys/dev/godot/godot3() [0xdd448c] (??:?)
[22] /home/nafys/dev/godot/godot3() [0x1aaca96] (??:?)
[23] /home/nafys/dev/godot/godot3() [0x1aad57d] (:?)
[24] /home/nafys/dev/godot/godot3() [0x2c2582f] (??:?)
[25] /home/nafys/dev/godot/godot3() [0xdd4cbb] (??:?)
[26] /home/nafys/dev/godot/godot3() [0xdbc54d] (??:?)
[27] /home/nafys/dev/godot/godot3() [0x1c0c1c5] (:?)
[28] /home/nafys/dev/godot/godot3() [0x1c29b44] (:?)
[29] /home/nafys/dev/godot/godot3() [0x1c2b515] (??:?)
[30] /home/nafys/dev/godot/godot3() [0x1c2b666] (:?)
[31] /home/nafys/dev/godot/godot3() [0x2c2582f] (??:?)
[32] /home/nafys/dev/godot/godot3() [0xde38f6] (??:?)
[33] /home/nafys/dev/godot/godot3() [0xdbc5d5] (??:?)
[34] /home/nafys/dev/godot/godot3() [0x1c4ed17] (??:?)
[35] /home/nafys/dev/godot/godot3() [0x1c62700] (??:?)
[36] /home/nafys/dev/godot/godot3() [0x2f04639] (??:?)
[37] /home/nafys/dev/godot/godot3() [0x2ea87ec] (:?)
[38] /home/nafys/dev/godot/godot3() [0x2f0721f] (:?)
[39] /home/nafys/dev/godot/godot3() [0x97b24c] (??:?)
[40] /usr/lib/libc.so.6(__libc_start_main+0xf2) [0x7faa71345152] (??:0)
[41] /home/nafys/dev/godot/godot3() [0x98997e] (??:?)
-- END OF BACKTRACE --

Steps to reproduce:
bug.zip

open the project
open the instanced scene in new tab
save it
return to main scene

@ghost
Copy link
Author

ghost commented Jan 10, 2021

Just checked with beta 4 and it work just fine

@qarmin
Copy link
Contributor

qarmin commented Jan 10, 2021

Not sure if this exactly same issue but running project(with GraphNodes) - wrp.zip and closing it cause Godot crash:

[1] /lib/x86_64-linux-gnu/libc.so.6(+0x46210) [0x7f41cbec5210] (??:0)
[2] Object::_disconnect(StringName const&, Object*, StringName const&, bool) (/mnt/Miecz/godot3.2/core/object.cpp:1569 (discriminator 1))
[3] Object::disconnect(StringName const&, Object*, StringName const&) (/mnt/Miecz/godot3.2/core/object.cpp:1561)
[4] GraphEdit::remove_child_notify(Node*) (/mnt/Miecz/godot3.2/scene/gui/graph_edit.cpp:426 (discriminator 4))
[5] Node::remove_child(Node*) (/mnt/Miecz/godot3.2/scene/main/node.cpp:1265)
[6] Node::_notification(int) (/mnt/Miecz/godot3.2/scene/main/node.cpp:175)
[7] Node::_notificationv(int, bool) (/mnt/Miecz/godot3.2/./scene/main/node.h:46 (discriminator 14))
[8] CanvasItem::_notificationv(int, bool) (/mnt/Miecz/godot3.2/./scene/2d/canvas_item.h:166)
[9] Control::_notificationv(int, bool) (/mnt/Miecz/godot3.2/./scene/gui/control.h:48)
[10] GraphEdit::_notificationv(int, bool) (/mnt/Miecz/godot3.2/scene/gui/graph_edit.h:98)
[11] Object::notification(int, bool) (/mnt/Miecz/godot3.2/core/object.cpp:931)
[12] Object::_predelete() (/mnt/Miecz/godot3.2/core/object.cpp:388)
[13] predelete_handler(Object*) (/mnt/Miecz/godot3.2/core/object.cpp:2057)
[14] void memdelete<Node>(Node*) (/mnt/Miecz/godot3.2/./core/os/memory.h:114)
[15] Node::_notification(int) (/mnt/Miecz/godot3.2/scene/main/node.cpp:171)
[16] Node::_notificationv(int, bool) (/mnt/Miecz/godot3.2/./scene/main/node.h:46 (discriminator 14))
[17] CanvasItem::_notificationv(int, bool) (/mnt/Miecz/godot3.2/./scene/2d/canvas_item.h:166)
[18] Control::_notificationv(int, bool) (/mnt/Miecz/godot3.2/./scene/gui/control.h:48)
[19] Object::notification(int, bool) (/mnt/Miecz/godot3.2/core/object.cpp:931)
[20] Object::_predelete() (/mnt/Miecz/godot3.2/core/object.cpp:388)
[21] predelete_handler(Object*) (/mnt/Miecz/godot3.2/core/object.cpp:2057)
[22] void memdelete<Node>(Node*) (/mnt/Miecz/godot3.2/./core/os/memory.h:114)
[23] Node::_notification(int) (/mnt/Miecz/godot3.2/scene/main/node.cpp:171)
[24] Node::_notificationv(int, bool) (/mnt/Miecz/godot3.2/./scene/main/node.h:46 (discriminator 14))
[25] Viewport::_notificationv(int, bool) (/mnt/Miecz/godot3.2/scene/main/viewport.h:91)
[26] Object::notification(int, bool) (/mnt/Miecz/godot3.2/core/object.cpp:931)
[27] Object::_predelete() (/mnt/Miecz/godot3.2/core/object.cpp:388)
[28] predelete_handler(Object*) (/mnt/Miecz/godot3.2/core/object.cpp:2057)
[29] void memdelete<Viewport>(Viewport*) (/mnt/Miecz/godot3.2/./core/os/memory.h:114)
[30] SceneTree::finish() (/mnt/Miecz/godot3.2/scene/main/scene_tree.cpp:622)
[31] OS_X11::run() (/mnt/Miecz/godot3.2/platform/x11/os_x11.cpp:3611)
[32] /usr/bin/godot(main+0x125) [0x138a58b] (/mnt/Miecz/godot3.2/platform/x11/godot_x11.cpp:57)
[33] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf3) [0x7f41cbea60b3] (??:0)
[34] /usr/bin/godot(_start+0x2e) [0x138a3ae] (??:?)

Address Sanitizer log

==19396==ERROR: AddressSanitizer: heap-use-after-free on address 0x61a00013a490 at pc 0x00000e850f92 bp 0x7ffd4390c8e0 sp 0x7ffd4390c8d0
READ of size 8 at 0x61a00013a490 thread T0
    #0 0xe850f91 in Object::_disconnect(StringName const&, Object*, StringName const&, bool) core/object.cpp:1569
    #1 0xe85055f in Object::disconnect(StringName const&, Object*, StringName const&) core/object.cpp:1560
    #2 0x9b48980 in GraphEdit::remove_child_notify(Node*) scene/gui/graph_edit.cpp:426
    #3 0x950cfd6 in Node::remove_child(Node*) scene/main/node.cpp:1264
    #4 0x94e65b8 in Node::_notification(int) scene/main/node.cpp:174
    #5 0x16e0143 in Node::_notificationv(int, bool) scene/main/node.h:46
    #6 0x16e2a54 in CanvasItem::_notificationv(int, bool) scene/2d/canvas_item.h:166
    #7 0x16e513e in Control::_notificationv(int, bool) scene/gui/control.h:48
    #8 0x9bc1cde in GraphEdit::_notificationv(int, bool) scene/gui/graph_edit.h:98
    #9 0xe83bc05 in Object::notification(int, bool) core/object.cpp:929
    #10 0xe82af23 in Object::_predelete() core/object.cpp:387
    #11 0xe8642d4 in predelete_handler(Object*) core/object.cpp:2056
    #12 0x1dc53c3 in void memdelete<Node>(Node*) core/os/memory.h:114
    #13 0x94e65c7 in Node::_notification(int) scene/main/node.cpp:175
    #14 0x16e0143 in Node::_notificationv(int, bool) scene/main/node.h:46
    #15 0x16e2a54 in CanvasItem::_notificationv(int, bool) scene/2d/canvas_item.h:166
    #16 0x16e513e in Control::_notificationv(int, bool) scene/gui/control.h:48
    #17 0xe83bc05 in Object::notification(int, bool) core/object.cpp:929
    #18 0xe82af23 in Object::_predelete() core/object.cpp:387
    #19 0xe8642d4 in predelete_handler(Object*) core/object.cpp:2056
    #20 0x1dc53c3 in void memdelete<Node>(Node*) core/os/memory.h:114
    #21 0x94e65c7 in Node::_notification(int) scene/main/node.cpp:175
    #22 0x16e0143 in Node::_notificationv(int, bool) scene/main/node.h:46
    #23 0x977f132 in Viewport::_notificationv(int, bool) scene/main/viewport.h:91
    #24 0xe83bc05 in Object::notification(int, bool) core/object.cpp:929
    #25 0xe82af23 in Object::_predelete() core/object.cpp:387
    #26 0xe8642d4 in predelete_handler(Object*) core/object.cpp:2056
    #27 0x9655bdd in void memdelete<Viewport>(Viewport*) core/os/memory.h:114
    #28 0x9601009 in SceneTree::finish() scene/main/scene_tree.cpp:621
    #29 0x1439299 in OS_X11::run() platform/x11/os_x11.cpp:3611
    #30 0x13a6b56 in main platform/x11/godot_x11.cpp:56
    #31 0x7fdce643c0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)
    #32 0x13a676d in _start (/usr/bin/godots+0x13a676d)

0x61a00013a490 is located 16 bytes inside of 1184-byte region [0x61a00013a480,0x61a00013a920)
freed by thread T0 here:
    #0 0x7fdce79011b7 in __interceptor_free (/lib/x86_64-linux-gnu/libasan.so.6+0xb01b7)
    #1 0xed77ae2 in Memory::free_static(void*, bool) core/os/memory.cpp:178
    #2 0x1dc5592 in void memdelete<Node>(Node*) core/os/memory.h:119
    #3 0x94e65c7 in Node::_notification(int) scene/main/node.cpp:175
    #4 0x16e0143 in Node::_notificationv(int, bool) scene/main/node.h:46
    #5 0x16e2a54 in CanvasItem::_notificationv(int, bool) scene/2d/canvas_item.h:166
    #6 0x16e513e in Control::_notificationv(int, bool) scene/gui/control.h:48
    #7 0x9bbc640 in GraphEditFilter::_notificationv(int, bool) scene/gui/graph_edit.h:46
    #8 0xe83bc05 in Object::notification(int, bool) core/object.cpp:929
    #9 0xe82af23 in Object::_predelete() core/object.cpp:387
    #10 0xe8642d4 in predelete_handler(Object*) core/object.cpp:2056
    #11 0x1dc53c3 in void memdelete<Node>(Node*) core/os/memory.h:114
    #12 0x94e65c7 in Node::_notification(int) scene/main/node.cpp:175
    #13 0x16e0143 in Node::_notificationv(int, bool) scene/main/node.h:46
    #14 0x16e2a54 in CanvasItem::_notificationv(int, bool) scene/2d/canvas_item.h:166
    #15 0x16e513e in Control::_notificationv(int, bool) scene/gui/control.h:48
    #16 0x9bc1cde in GraphEdit::_notificationv(int, bool) scene/gui/graph_edit.h:98
    #17 0xe83bc05 in Object::notification(int, bool) core/object.cpp:929
    #18 0xe82af23 in Object::_predelete() core/object.cpp:387
    #19 0xe8642d4 in predelete_handler(Object*) core/object.cpp:2056
    #20 0x1dc53c3 in void memdelete<Node>(Node*) core/os/memory.h:114
    #21 0x94e65c7 in Node::_notification(int) scene/main/node.cpp:175
    #22 0x16e0143 in Node::_notificationv(int, bool) scene/main/node.h:46
    #23 0x16e2a54 in CanvasItem::_notificationv(int, bool) scene/2d/canvas_item.h:166
    #24 0x16e513e in Control::_notificationv(int, bool) scene/gui/control.h:48
    #25 0xe83bc05 in Object::notification(int, bool) core/object.cpp:929
    #26 0xe82af23 in Object::_predelete() core/object.cpp:387
    #27 0xe8642d4 in predelete_handler(Object*) core/object.cpp:2056
    #28 0x1dc53c3 in void memdelete<Node>(Node*) core/os/memory.h:114
    #29 0x94e65c7 in Node::_notification(int) scene/main/node.cpp:175

previously allocated by thread T0 here:
    #0 0x7fdce7901517 in malloc (/lib/x86_64-linux-gnu/libasan.so.6+0xb0517)
    #1 0xed76bd5 in Memory::alloc_static(unsigned long, bool) core/os/memory.cpp:82
    #2 0xed76ad4 in operator new(unsigned long, char const*) core/os/memory.cpp:42
    #3 0x9bb5458 in GraphEdit::GraphEdit() scene/gui/graph_edit.cpp:1785
    #4 0x944aecd in Object* ClassDB::creator<GraphEdit>() (/usr/bin/godots+0x944aecd)
    #5 0xe5c4631 in ClassDB::instance(StringName const&) core/class_db.cpp:559
    #6 0xc21173e in SceneState::instance(SceneState::GenEditState) const scene/resources/packed_scene.cpp:157
    #7 0xc244b39 in PackedScene::instance(PackedScene::GenEditState) const scene/resources/packed_scene.cpp:1698
    #8 0x153f36c in Main::start() main/main.cpp:1943
    #9 0x13a6a89 in main platform/x11/godot_x11.cpp:55
    #10 0x7fdce643c0b2 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x270b2)

@akien-mga akien-mga added this to the 3.2 milestone Jan 11, 2021
@akien-mga
Copy link
Member

Possibly related to #44470? CC @pycbouh @Chaosus

@Chaosus
Copy link
Member

Chaosus commented Jan 11, 2021

@nafysbrhm Please attach a test project. I'm unable to reproduce this crash on my Windows 10 PC. Maybe it's related to the Linux subsystem.

@ghost
Copy link
Author

ghost commented Jan 11, 2021

@nafysbrhm Please attach a test project. I'm unable to reproduce this crash on my Windows 10 PC. Maybe it's related to the Linux subsystem.

I updated the post with the test project

@Chaosus
Copy link
Member

Chaosus commented Jan 11, 2021

Yep, thanks, it's still not crashing for me.
Its printing

E 0:00:03.137   _disconnect: Attempt to disconnect a nonexistent connection to signal 'item_rect_changed' in [GraphNode:1212], with target 'update' in [Object:0].
  <C++ Error>   Condition "signal_is_valid" is true.
  <C++ Source>  core/object.cpp:1569 @ _disconnect()

to the output however

@YuriSizov
Copy link
Contributor

YuriSizov commented Jan 11, 2021

E 0:00:03.137 _disconnect: Attempt to disconnect a nonexistent connection to signal 'item_rect_changed' in [GraphNode:1212], with target 'update' in [Object:0].
<C++ Error> Condition "signal_is_valid" is true.
<C++ Source> core/object.cpp:1569 @ _disconnect()

I'd seen this error when working on the minimap and double-checked it without my changes, and it was still happening. I assumed that to be an existing issue. And it didn't end up in a crash indeed. Maybe there is something else.

@YuriSizov
Copy link
Contributor

YuriSizov commented Jan 12, 2021

Editor crashed when working with GraphEdit and instanced GraphNode. Saving a scene with GraphNode as a base node, and return to the scene where it was instanced, make the editor crashed.

Indeed, this exact order of actions causes something like _disconnect: Attempt to disconnect a nonexistent connection to signal 'item_rect_changed' in [GraphNode:1212], with target 'update' in [Object:0]. on Windows, but no crash. And indeed with the minimap this issue became more severe, but the bug still exists in beta4. I can only reproduce it in beta4 for a running project, not the editor so far.

You need to cause the connections_layer of the GraphNode to update to produce it. For example, you can have two connected nodes and move one of them. Then close the running project, and the same error should be printed in the debugger. It was reported to me by another Linux user that the running project doesn't produce any crash, just that same error in the debugger.

In beta5 this error is thrown for every node once (because of the minimap) and then for every node that caused the connections_layer to update (so for 2 nodes in the example above it would be 3 errors).

image

The error is happening in remove_child_notify of GraphEdit, but it is correctly paired with add_child_notify, and there are 2 other signals that don't cause any errors. I will have to investigate this.


Same issue in the master, but a different message

 E 0:00:06:0437   Object::_disconnect: Condition "!target_object" is true.
  <C++ Source>   core\object\object.cpp:1357 @ Object::_disconnect()

@YuriSizov YuriSizov self-assigned this Jan 12, 2021
@YuriSizov
Copy link
Contributor

YuriSizov commented Jan 12, 2021

This regression was introduced by #41995, but my work on minimap exposed it. The situation is as follows: by the time GraphEdit::remove_child_notify is called, connections_layer and now minimap properties referencing corresponding child nodes are being freed. They still have a reference to an object, but that's an [Object:0], not a valid reference.

So attempting to call

	gn->disconnect("item_rect_changed", connections_layer, "update");
	gn->disconnect("item_rect_changed", minimap, "update");

causes an error, because there are no records of this signal being connected to an non-existent node. It doesn't matter what signal is being disconnected, the problem is that references are invalid. This is what happens in a running project:

add_child connecting [GraphEditMinimap:1210] in [GraphEdit:1193]
add_child connecting [GraphEditMinimap:1210] in [GraphEdit:1193]
remove_child disconnecting [Object:0] in [GraphEdit:1193]
remove_child disconnecting [Object:0] in [GraphEdit:1193]

Same happens in the editor, though additionally a new instance of the scene is created at the same time when changing scenes:

add_child connecting [GraphEditMinimap:28365] in [GraphEdit:28348]
add_child connecting [GraphEditMinimap:28365] in [GraphEdit:28348]
Switch Scene Tab
Switch Scene Tab
add_child connecting [GraphEditMinimap:32033] in [GraphEdit:32016]
add_child connecting [GraphEditMinimap:32033] in [GraphEdit:32016]
remove_child disconnecting [Object:0] in [GraphEdit:28348]
 Attempt to disconnect a nonexistent connection to signal 'raise_request' in [GraphNode:28368], with target 'dud' in [Object:0].
remove_child disconnecting [Object:0] in [GraphEdit:28348]
 Attempt to disconnect a nonexistent connection to signal 'raise_request' in [GraphNode:28366], with target 'dud' in [Object:0].

Since #41995 makes it clear, that the disconnect is required for some other issues, I can just propose to check if the instances are valid and only disconnect if they are. I assume, if the instances are being freed, disconnections should already be handled gracefully, just like in scripting.

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