Switching scene tabs duplicates nodes in the scene and eventually also leads to an editor crash.

[viksl]

Tested versions

• Reproducible: 4.3dev master[99ff024]
• Not reproducible: 4.3 dev5

System information

Windows 11 - Vulkan - Nvidia RTX 4070 - intel i5 13600KF

Issue description

When switching scene tabs while running the project some nodes in the scene tree get duplicated and stay in, eventually leading to a crash after couple times of repeating.

Video: https://youtu.be/11MNyMQJlh0

Steps to reproduce

1. Open the MRP.
2. Open the Test.tscn to have two tabs (main and test).
3. Go to main and press save and reload the project (this step is just in case, might not be necessary).
4. Play the project and keep the play window somewhere (doesn't matter).
5. In the editor switch from main to test tab and then back to the main tab.
6. Observe some nodes got duplicated.
   (7. Repeat this proces to duplicate more - you might need to always press the stop and play buttons for every duplication - and observe after couple duplications the editor hard crashes)

Minimal reproduction project (MRP)

Mrp.zip

EDIT(@lyuma + @Illauriel): simpler MRP and repro steps:
Another MRP: node-dupe-bug.zip
Steps to reproduce:

1. Open node3d.tscn.
2. Open the nested scene.
3. Save the nested scene
4. Return to the original node3d.tscn. it will contain duplicate nodes.

The reason play duplicates nodes is because the play button saves all scenes. Therefore saving is sufficient.

[AThousandShips]

Can confirm will try bisect in a bit

[AThousandShips]

Regression from:

• Fix error where update wouldn't reconnect the signal it should have reconnected and led to errors and crash. #89447

Unsure if this caused it or just exposed an existing issue that didn't fire without this

CC @ajreckof @KoBeWi

[KoBeWi]

You don't need to play the project. Just save the instance scene and go back to the parent scene. This causes reloading the scene (to update the instance) and leads to the mentioned bug. The weird part is that it duplicates only one node from the scene 🤔

After enough repeats, I got the editor to freeze while spamming Object was deleted while awaiting a callback. in the console.

[viksl]

In my own project (not the MRP) it duplicates several nodes as seen here (check the Time label is the only one without @ in the name but it was also duplicated).
In the image my scene ends with the CanvasLayer-Time anything below gets duplicated, all duplicated at the same time (that's four nodes every time I switch tabs).

It happens to me always only once after I play the project then I need to stop and play agian, switch scenes again and it duplicates all the nodes again (but more switching doesn't duplicate anything), then I can do it one more time and if I try to do it for the fourth time it always crashes with this:

================================================================
CrashHandlerException: Program crashed
Engine version: Godot Engine v4.3.dev.mono.custom_build (99ff024f78f65ba0bc54fb409cfeca43ba2008fe)
Dumping the backtrace. Please include this when reporting the bug to the project developer.
[0] std::_Atomic_integral<unsigned __int64,8>::fetch_add (E:\Program Files\Microsoft Visual Studio\2022\Community\VC\To)
[1] std::_Atomic_integral_facade<unsigned __int64>::fetch_sub (E:\Program Files\Microsoft Visual Studio\2022\Community\)
[2] SafeNumeric<unsigned __int64>::decrement (E:\Projects\Godot\godot-viksl\godot-viksl\core\templates\safe_refcount.h:)
[3] CowData<Plane>::_unref (E:\Projects\Godot\godot-viksl\godot-viksl\core\templates\cowdata.h:252)
[4] CowData<Plane>::_ref (E:\Projects\Godot\godot-viksl\godot-viksl\core\templates\cowdata.h:464)
[5] Vector<Plane>::operator= (E:\Projects\Godot\godot-viksl\godot-viksl\core\templates\vector.h:149)
[6] RendererSceneCull::Frustum::operator= (E:\Projects\Godot\godot-viksl\godot-viksl\servers\rendering\renderer_scene_c)
[7] RendererSceneCull::_light_instance_setup_directional_shadow (E:\Projects\Godot\godot-viksl\godot-viksl\servers\rend)
[8] RendererSceneCull::_render_scene (E:\Projects\Godot\godot-viksl\godot-viksl\servers\rendering\renderer_scene_cull.c)
[9] RendererSceneCull::render_camera (E:\Projects\Godot\godot-viksl\godot-viksl\servers\rendering\renderer_scene_cull.c)
[10] RendererViewport::_draw_3d (E:\Projects\Godot\godot-viksl\godot-viksl\servers\rendering\renderer_viewport.cpp:250)
[11] RendererViewport::_draw_viewport (E:\Projects\Godot\godot-viksl\godot-viksl\servers\rendering\renderer_viewport.cp)
[12] RendererViewport::draw_viewports (E:\Projects\Godot\godot-viksl\godot-viksl\servers\rendering\renderer_viewport.cp)
[13] RenderingServerDefault::_draw (E:\Projects\Godot\godot-viksl\godot-viksl\servers\rendering\rendering_server_defaul)
[14] RenderingServerDefault::draw (E:\Projects\Godot\godot-viksl\godot-viksl\servers\rendering\rendering_server_default)
[15] Main::iteration (E:\Projects\Godot\godot-viksl\godot-viksl\main\main.cpp:4022)
[16] OS_Windows::run (E:\Projects\Godot\godot-viksl\godot-viksl\platform\windows\os_windows.cpp:1476)
[17] widechar_main (E:\Projects\Godot\godot-viksl\godot-viksl\platform\windows\godot_windows.cpp:181)
[18] _main (E:\Projects\Godot\godot-viksl\godot-viksl\platform\windows\godot_windows.cpp:206)
[19] main (E:\Projects\Godot\godot-viksl\godot-viksl\platform\windows\godot_windows.cpp:220)
[20] WinMain (E:\Projects\Godot\godot-viksl\godot-viksl\platform\windows\godot_windows.cpp:234)
[21] __scrt_common_main_seh (D:\a\_work\1\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
[22] <couldn't map PC to fn name>
-- END OF BACKTRACE --
================================================================

But this is from my own project not MRP, it's also from mono version so not sure if the error is even related at all.

[KoBeWi]

Well the crash seems specific to your scene. The bug is that nodes get duplicated, but depending on nodes, the end result might be different.

[AThousandShips]

It's due to an incorrect deletion sequence in the PR, writing a fix right now:

godot/editor/editor_data.cpp

Lines 725 to 727 in 2850ce2

	for (int i = 0; i < old_root->get_child_count(); i++) {
	memdelete(old_root->get_child(i));
	}

[akien-mga]

Indeed, this should go from last to first.

But this approach feels hacky in the first place, maybe we could add a bool to replace_by to not copy child nodes?

[AThousandShips]

Indeed, will leave making a better fix for now, could push a trivial fix but sounds like this might be an issue with ownership in replaced nodes or similar, so let's see what deeper solution is needed

[lyuma]

@Illauriel and I confirmed a bisect of #89447
Another MRP: node-dupe-bug.zip

Steps to reproduce:

1. Open node3d.tscn.
2. Open the nested scene.
3. Save the nested scene
4. Return to the original node3d.tscn. it will contain duplicate nodes.

I also had a suspicion that an internal node somewhere could cause an issue: I recall from debugging a different issue that the core function Node::replace_by may be buggy in the presence of internal nodes (off-by-n error), since it does not properly check indexing, so seeing replace_by in the regression PR is setting off that alarm bell as well.

From my notes about replace_by:

All nodes have to be added and moved to the same index, one by one. This might make files with thousands of nodes slow.
Calling replace_by on an internal node will malfunction because it does not preserve the internal_node in the add_child call
I do think there's a core bug in that replace_by is incorrect for internal nodes

I should try and repro + file the bug about replace_by, since it's probably a separate issue, but wanted to bring it up anyway just in case.

[Maran23]

Moving a GDScript from one folder to another also reproduced this bug for me.

[Rindbee]

It seems that it will appear as long as you save the sub-scene and switch to the main scene.

Nodes will be duplicated only when the nodes meet the following conditions:

1. Is a direct non-internal child node of the main scene root;
2. The child node index is an even number. (This may be due to the order in which nodes are deleted)