godotengine / godot

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

Changing a MeshInstance3D's mesh to a certain new meshes crashes Godot, in certain projects #63555

Open Braveo opened 2 years ago

Braveo commented 2 years ago

Godot version

4.0.alpha12

System information

Windows 10 64 bit, 11th Gen Intel(R) i5-1135G7 @2.40GHz, Intel iRISx(R) Graphics, Vulkan

Issue description

I had a scene with 3 mesh instances which were spheres. Then, I duplicated one and changed the mesh to "New Cylinder" only for Godot to crash.

There was a workaround to this, which was pretty much deleting the MeshInstance3D and creating a new one, but I found this to be an issue nonetheless.


After creating a new project file, I tried to reproduce the effects, but nothing happened. It only happened to the project file I was working with, so I'll send that just in case the project's specific settings may have something to do with it:

The scene where it was reproduced is in Test.tcsn: Godot4Test.zip

Steps to reproduce

1 Create an empty scene with a mesh instance, and create a mesh (which was a sphere for me). 2 Duplicate one of the spheres and change the mesh to a cylinder. (it also happened with prism mesh to me). 3 The software should crash, at least it did for me.

Minimal reproduction project

No response

allenwp commented 2 years ago

I was able to reproduce this using the project attached in the original issue. Here is a video of how I reproduced it. (Note: I was using the CTRL-D keyboard shortcut to duplicate the spheres before changing their mesh to cylinder)

https://user-images.githubusercontent.com/17506573/181520450-d169a8d6-3a70-4b73-8573-de84e6dbd826.mp4

I am just about to boot up a debug build of the editor to see if I can capture a crash with the visual studio debugger...

allenwp commented 2 years ago

I was able to reproduce with the visual studio debugger attached to a local build of revision 2c11e6d9efc42370a8d7537eaff8b1ea78a283e5 (what alpha 12 was built off of).

Here are the crash details:

Exception thrown: read access violation.
**mtx** was 0x218.

Call stack:

godot.windows.tools.64.exe!mtx_do_lock(_Mtx_internal_imp_t * mtx, const xtime * target) Line 91 C++
godot.windows.tools.64.exe!std::_Mutex_base::lock() Line 50 C++
godot.windows.tools.64.exe!MutexImpl<std::recursive_mutex>::lock() Line 48  C++
godot.windows.tools.64.exe!MutexLock<MutexImpl<std::recursive_mutex>>::MutexLock<MutexImpl<std::recursive_mutex>>(const MutexImpl<std::recursive_mutex> & p_mutex) Line 67  C++
godot.windows.tools.64.exe!SceneTree::queue_delete(Object * p_object) Line 1074 C++
godot.windows.tools.64.exe!Node::queue_delete() Line 2638   C++
godot.windows.tools.64.exe!Viewport::_gui_cancel_tooltip() Line 1128    C++
godot.windows.tools.64.exe!Viewport::_gui_input_event(Ref<InputEvent> p_event) Line 1661    C++
godot.windows.tools.64.exe!Viewport::push_input(const Ref<InputEvent> & p_event, bool p_local_coords) Line 2721 C++
godot.windows.tools.64.exe!Window::_window_input(const Ref<InputEvent> & p_ev) Line 1021    C++
godot.windows.tools.64.exe!call_with_variant_args_helper<Window,Ref<InputEvent> const &,0>(Window * p_instance, void(Window::*)(const Ref<InputEvent> &) p_method, const Variant * * p_args, Callable::CallError & r_error, IndexSequence<0> __formal) Line 259 C++
godot.windows.tools.64.exe!call_with_variant_args<Window,Ref<InputEvent> const &>(Window * p_instance, void(Window::*)(const Ref<InputEvent> &) p_method, const Variant * * p_args, int p_argcount, Callable::CallError & r_error) Line 374 C++
godot.windows.tools.64.exe!CallableCustomMethodPointer<Window,Ref<InputEvent> const &>::call(const Variant * * p_arguments, int p_argcount, Variant & r_return_value, Callable::CallError & r_call_error) Line 105  C++
godot.windows.tools.64.exe!Callable::call(const Variant * * p_arguments, int p_argcount, Variant & r_return_value, Callable::CallError & r_call_error) Line 51  C++
godot.windows.tools.64.exe!DisplayServerWindows::_dispatch_input_event(const Ref<InputEvent> & p_event) Line 2079   C++
godot.windows.tools.64.exe!DisplayServerWindows::_dispatch_input_events(const Ref<InputEvent> & p_event) Line 2044  C++
godot.windows.tools.64.exe!Input::_parse_input_event_impl(const Ref<InputEvent> & p_event, bool p_is_emulated) Line 663 C++
godot.windows.tools.64.exe!Input::flush_buffered_events() Line 889  C++
godot.windows.tools.64.exe!DisplayServerWindows::process_events() Line 1786 C++
godot.windows.tools.64.exe!OS_Windows::run() Line 777   C++
godot.windows.tools.64.exe!widechar_main(int argc, wchar_t * * argv) Line 179   C++
godot.windows.tools.64.exe!_main() Line 201 C++
godot.windows.tools.64.exe!main(int argc, char * * argv) Line 215   C++
godot.windows.tools.64.exe!WinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, char * lpCmdLine, int nCmdShow) Line 229   C++
[External Code]

https://user-images.githubusercontent.com/17506573/185236567-57404dfa-ddb9-48d8-b35a-68b71963cd27.mp4

I was also able to reproduce the crash with an identical exception and stack trace with commit 82811367cb36d3124d4e8c0a9c4c7f82dc64f9e4.

I might be able to look at this later, but if I do it will be my first contribution to the project, so I'll need to spend some time learning code conventions and how to do a PR properly, etc. and figure out what's going on with this crash and how to correctly fix it. All that to say, if the fix is obvious to someone else, please let me know...

EDIT: actually, I think I'm going to bail on trying to fix this myself... multi-threading and mutexes are something I have no experience with, so I'm not sure how this whole _THREAD_SAFE_METHOD_ thing is supposed to behave in the first place. I hope those details I was able to get are helpful!

allenwp commented 2 years ago

Also, thanks for the bug report and reproduction project @Braveo 😁

allenwp commented 2 years ago

This bug seems very difficult to reproduce. It's rare that I can get it to happen given a debug build with a debugger attached. At times it seems to happen around 25% of the time that I switch meshes. Other times it happens only 1-5% of the time. I have had the crash happen when switching to box meshes or capsule meshes as well. I've also had it happen without duplicating the sphere first: I just open the scene, switch the mesh, and it crashes. I have also had it happen when the mesh selection drop-down menu is appearing over top of different scripts in the inspector (for example, it happens when the "cylinder" mesh option appears over the VisualInstance3D section of the inspector... And it also happens when the option appears over MeshInstance3D part of the inspector). I've got video recordings of all these scenarios.

The reason I mention all of this is because I haven't been able to reproduce the crash even once with alpha 13 that I downloaded today (Aug 3, 2022). Is it possible that this crash was fixed sometime between 82811367cb36d3124d4e8c0a9c4c7f82dc64f9e4 and alpha 13?

allenwp commented 2 years ago

Another update on this bug:

I'm a bit baffled because I expected 82811367cb36d3124d4e8c0a9c4c7f82dc64f9e4 to behave similar to v4.0.alpha13.official [59dcddc45]... Unless the hotfix update of v4.0.alpha13.official [59dcddc45] includes more commits past 82811367cb36d3124d4e8c0a9c4c7f82dc64f9e4 than the hotfix, which I assumed was just cherry picked.

Anyway, maybe this bug was fixed as of v4.0.alpha13.official [59dcddc45], but it's so inconsistent to reproduce that I really can't give myself confidence.

One other thing to note: I've also tried using AutoHotkey for Windows to automate mesh changing, but even after an hour of letting it run, it won't reproduce the crash on v4.0.alpha12.official [2c11e6d9e], even though I am able to reproduce it by hand without much trouble. It seems that there is something to do with human mouse movements and clicks that causes the bug. Maybe something with exactly which frame you release the mouse button on or if there is any mouse movement between mouse up and mouse down... I've tried for a while, but this is a real head scratcher...

allenwp commented 2 years ago

I tried letting the AutoHotkey automation run for 4 hours with v4.0.alpha12.official [2c11e6d9e]. As expected, no crash. I stopped the automation and manually changed the mesh and it crashed first time. This is the second time I saw this exact behaviour of no crash from the automation and a crash upon the first manual mesh swap following the automation. There's something to do with human mouse usage at play that I wasn't able to simulate with an AutoHotkey script...?

https://user-images.githubusercontent.com/17506573/185268610-4bbaa84d-19fa-4a20-b98d-bf4315b66864.mp4

I have tried the same approach with v4.0.alpha14.official [106b68050] and could not reproduce the crash.

I've been trying to reproduce with a alpha 13 and 14 for a while now, using various approaches and have never been able to reproduce it, so I'm confident that this bug has now been resolved.