Crash on CowData - Githubissues

BastiaanOlij commented 5 years ago

Godot version: Godot build accc51f3012e0010eba2c56b8ed48c11a0fee3a7 GDProcMesh build 39d39ab74ea4ee06150970e26e004f408e99652b

OS/device including version: Windows 10, GTX 1060

Issue description: I don't know if this is specific to my ProcMesh GDNative module or an issue in Godot itself but it's a weird one.

ProcMesh basically uses ArrayMesh to generate meshes. Meshes I've already generated previously work fine, I can alter them, regenerate them, etc. and it all goes well.

But starting with an empty ArrayMesh, setting ProcMesh as the script and then generating a simple box (or anything really) crashes Godot with the following:

OpenGL ES 3.0 Renderer: GeForce GTX 1060/PCIe/SSE2
ERROR: CowData<class RID>::get: FATAL: Index p_index=0 out of size (size()=0)
   At: C:\Users\basti\Development\godot3-git\core/cowdata.h:149
CrashHandlerException: Program crashed
Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[0] CowData<RID>::get (c:\users\basti\development\godot3-git\core\cowdata.h:149)
[1] Vector<RID>::operator[] (c:\users\basti\development\godot3-git\core\vector.h:85)
[2] RasterizerSceneGLES3::_fill_render_list (c:\users\basti\development\godot3-git\drivers\gles3\rasterizer_scene_gles3.cpp:3170)
[3] RasterizerSceneGLES3::render_scene (c:\users\basti\development\godot3-git\drivers\gles3\rasterizer_scene_gles3.cpp:4147)
[4] VisualServerScene::_render_scene (c:\users\basti\development\godot3-git\servers\visual\visual_server_scene.cpp:2145)
[5] VisualServerScene::render_camera (c:\users\basti\development\godot3-git\servers\visual\visual_server_scene.cpp:1714)
[6] VisualServerViewport::_draw_viewport (c:\users\basti\development\godot3-git\servers\visual\visual_server_viewport.cpp:70)
[7] VisualServerViewport::draw_viewports (c:\users\basti\development\godot3-git\servers\visual\visual_server_viewport.cpp:308)
[8] VisualServerRaster::draw (c:\users\basti\development\godot3-git\servers\visual\visual_server_raster.cpp:107)
[9] VisualServerWrapMT::draw (c:\users\basti\development\godot3-git\servers\visual\visual_server_wrap_mt.cpp:104)
[10] Main::iteration (c:\users\basti\development\godot3-git\main\main.cpp:1874)
[11] OS_Windows::run (c:\users\basti\development\godot3-git\platform\windows\os_windows.cpp:2754)
[12] widechar_main (c:\users\basti\development\godot3-git\platform\windows\godot_win.cpp:150)
[13] _main (c:\users\basti\development\godot3-git\platform\windows\godot_win.cpp:172)
[14] main (c:\users\basti\development\godot3-git\platform\windows\godot_win.cpp:184)
[15] __scrt_common_main_seh (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:283)
[16] BaseThreadInitThunk
-- END OF BACKTRACE --

Procmesh uses Pools extensively so I'm wondering if the recent changes to how COW is handled has something to do with it. Specifically if some of the pools in the surface array are empty/nil ?

Steps to reproduce: Easiest way is to clone my current procmesh master: https://github.com/BastiaanOlij/gdprocmesh It has a compiled 64bit binary and a demo project. Just use the test.tscn then:

select the meshinstance
select the associated mesh
click on the edit procmesh graph button and hook up the output of the "box" node to the input of the "output" node

Minimal reproduction project: https://github.com/BastiaanOlij/gdprocmesh

BastiaanOlij commented 5 years ago

May or may not be related but I just had a similar crash but this time unrelated to GDNative. This was exporting a meshlib:

created a few simple shapes in Blender
exported with better collada
opened dae in Godot
export to meshlib -> crashes

CrashHandlerException: Program crashed
Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[0] CowData<Ref<Image> >::get (c:\users\basti\development\godot3-git\core\cowdata.h:149)
[1] Vector<Ref<Image> >::operator[] (c:\users\basti\development\godot3-git\core\vector.h:85)
[2] RasterizerStorageGLES3::texture_get_data (c:\users\basti\development\godot3-git\drivers\gles3\rasterizer_storage_gles3.cpp:1036)
[3] VisualServerRaster::texture_get_data (c:\users\basti\development\godot3-git\servers\visual\visual_server_raster.h:153)
[4] VisualServerWrapMT::texture_get_data (c:\users\basti\development\godot3-git\servers\visual\visual_server_wrap_mt.h:88)
[5] EditorInterface::make_mesh_previews (c:\users\basti\development\godot3-git\editor\editor_plugin.cpp:129)
[6] MeshLibraryEditor::_import_scene (c:\users\basti\development\godot3-git\editor\plugins\mesh_library_editor_plugin.cpp:163)
[7] MeshLibraryEditor::update_library_file (c:\users\basti\development\godot3-git\editor\plugins\mesh_library_editor_plugin.cpp:186)
[8] EditorNode::_dialog_action (c:\users\basti\development\godot3-git\editor\editor_node.cpp:1251)
[9] MethodBind1<EditorNode,String>::call (c:\users\basti\development\godot3-git\core\method_bind.gen.inc:815)
[10] Object::call (c:\users\basti\development\godot3-git\core\object.cpp:945)
[11] Object::emit_signal (c:\users\basti\development\godot3-git\core\object.cpp:1231)
[12] Object::emit_signal (c:\users\basti\development\godot3-git\core\object.cpp:1288)
[13] EditorFileDialog::_action_pressed (c:\users\basti\development\godot3-git\editor\editor_file_dialog.cpp:445)
[14] EditorFileDialog::_file_entered (c:\users\basti\development\godot3-git\editor\editor_file_dialog.cpp:231)
[15] MethodBind1<EditorFileDialog,String const &>::call (c:\users\basti\development\godot3-git\core\method_bind.gen.inc:815)
[16] Object::call (c:\users\basti\development\godot3-git\core\object.cpp:945)
[17] Object::emit_signal (c:\users\basti\development\godot3-git\core\object.cpp:1231)
[18] Object::emit_signal (c:\users\basti\development\godot3-git\core\object.cpp:1288)
[19] LineEdit::_gui_input (c:\users\basti\development\godot3-git\scene\gui\line_edit.cpp:269)
[20] MethodBind1<LineEdit,Ref<InputEvent> >::call (c:\users\basti\development\godot3-git\core\method_bind.gen.inc:815)
[21] Object::call_multilevel (c:\users\basti\development\godot3-git\core\object.cpp:779)
[22] Object::call_multilevel (c:\users\basti\development\godot3-git\core\object.cpp:886)
[23] Viewport::_gui_input_event (c:\users\basti\development\godot3-git\scene\main\viewport.cpp:2209)
[24] Viewport::input (c:\users\basti\development\godot3-git\scene\main\viewport.cpp:2608)
[25] Viewport::_vp_input (c:\users\basti\development\godot3-git\scene\main\viewport.cpp:1293)
[26] MethodBind1<Viewport,Ref<InputEvent> const &>::call (c:\users\basti\development\godot3-git\core\method_bind.gen.inc:815)
[27] Object::call (c:\users\basti\development\godot3-git\core\object.cpp:945)
[28] Object::call (c:\users\basti\development\godot3-git\core\object.cpp:869)
[29] SceneTree::call_group_flags (c:\users\basti\development\godot3-git\scene\main\scene_tree.cpp:262)
[30] SceneTree::input_event (c:\users\basti\development\godot3-git\scene\main\scene_tree.cpp:418)
[31] InputDefault::_parse_input_event_impl (c:\users\basti\development\godot3-git\main\input_default.cpp:440)
[32] InputDefault::parse_input_event (c:\users\basti\development\godot3-git\main\input_default.cpp:260)
[33] OS_Windows::process_key_events (c:\users\basti\development\godot3-git\platform\windows\os_windows.cpp:1008)
[34] OS_Windows::process_events (c:\users\basti\development\godot3-git\platform\windows\os_windows.cpp:2200)
[35] OS_Windows::run (c:\users\basti\development\godot3-git\platform\windows\os_windows.cpp:2754)
[36] widechar_main (c:\users\basti\development\godot3-git\platform\windows\godot_win.cpp:150)
[37] _main (c:\users\basti\development\godot3-git\platform\windows\godot_win.cpp:172)
[38] main (c:\users\basti\development\godot3-git\platform\windows\godot_win.cpp:184)
[39] __scrt_common_main_seh (f:\dd\vctools\crt\vcstartup\src\startup\exe_common.inl:283)
[40] BaseThreadInitThunk
-- END OF BACKTRACE --

BastiaanOlij commented 5 years ago

From IRC:

tmm
05:38
Mux213 ERROR: CowData<class RID>::get: FATAL: Index p_index=0 out of size (size()=0) <--- that is supposed to crash

Mux213
21:10
tmm: yeah but why is it zero when exporting a few objects to a meshlib? Only thing I can think of is that a mesh has a few optional arrays that may indeed be empty
tmm: and that could very well mean we're looking at an issue thats been in the system for awhile ;)
tmm: but it could also mean its not checking it properly and not handling that those indeed may be empty

BastiaanOlij commented 5 years ago

Not sure if this will explain the second crash but the first crash is caused by the materials array on the MeshInstance node not being resized to the number of surfaces in the mesh. If I read this correctly when the mesh is assigned or when the mesh is changed a changed signal should be emitted that leads up to MeshInstance::_mesh_changed being called. This in turn resizes the materials array to match.

With an incorrectly sized materials array the render code breaks on: https://github.com/godotengine/godot/blob/master/drivers/gles3/rasterizer_scene_gles3.cpp#L3170

int ssize = mesh->surfaces.size();

for (int i = 0; i < ssize; i++) {
    int mat_idx = inst->materials[i].is_valid() ? i : -1;
    RasterizerStorageGLES3::Surface *s = mesh->surfaces[i];
    _add_geometry(s, inst, NULL, mat_idx, p_depth_pass, p_shadow_pass);
}

Now as far as I can tell the signal is emitted when ArrayMesh::add_surface_from_arrays is called (which is what drives procmesh).

Could this be a timing issue? Where the scene is rendered before the signal was processed?

BastiaanOlij commented 5 years ago

It is a timing issue but its not related to the signals, there are a good 5 to 6 frames rendered where the materials array in the mesh instance has been resized but the array stored within the visual server is still empty.

@reduz any ideas?

I'll at least put a size check in the code that crashes.

akien-mga commented 5 years ago

Note: the second crash is fixed by #24651. The first crash (OP) is worked around by #24650, but @reduz would like to have a closer look at what the actual cause may be.

akien-mga commented 5 years ago

The first crash seems to be a duplicate of #23790. Thus closing this issue as the second crash mentioned is fixed, so it's clearer this way.

@BastiaanOlij Could you copy the relevant debug info and steps to reproduce to #23790?

godotengine / godot

Crash on CowData #24635