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

Crash in EditorFileSystem scan due to reentrant call through Main::iteration #46893

Closed
lyuma opened this issue Mar 11, 2021 · 2 comments
Closed

Comments

@lyuma
Copy link
Contributor

lyuma commented Mar 11, 2021

Godot version:
4.0.dev 01851de

( Also reproduced on a build from Feb. 20, 2021. Did not test other versions )

OS/device including version:
Windows 10.0.19041.804

Issue description:
I have a tool script which opens a popup dialong. When the user clicks OK, my GDScript creates png, glb files, and other importable assets. Some assets have a custom EditorImportPlugin.

After creating the files, the tool script uses a Timer node to add a delay (to avoid any possibility of my tool script causing reentrancy into the importer), followed by calling EditorFileSystem::scan()

After the tool script completes execution and scan() is called, the Godot editor starts importing the newly created files showing the progress bar). Some assets are imported before the progress bar shows. During the process, this crash occurs.

	godot.windows.tools.64.exe!CowData<char32_t>::size() Line 135	C++
 	godot.windows.tools.64.exe!String::size() Line 214	C++
 	godot.windows.tools.64.exe!String::length() Line 268	C++
 	godot.windows.tools.64.exe!String::operator==(const String & p_str) Line 580	C++
 	godot.windows.tools.64.exe!EditorFileSystemDirectory::find_file_index(const String & p_file) Line 54	C++
 	godot.windows.tools.64.exe!EditorFileSystem::_update_scan_actions() Line 547	C++
 	godot.windows.tools.64.exe!EditorFileSystem::_notification(int p_what) Line 1137	C++
 	godot.windows.tools.64.exe!EditorFileSystem::_notificationv(int p_notification, bool p_reversed) Line 108	C++
 	godot.windows.tools.64.exe!Object::notification(int p_notification, bool p_reversed) Line 795	C++
 	godot.windows.tools.64.exe!SceneTree::_notify_group_pause(const StringName & p_group, int p_notification) Line 810	C++
 	godot.windows.tools.64.exe!SceneTree::process(float p_time) Line 442	C++
 	godot.windows.tools.64.exe!Main::iteration() Line 2489	C++
 	godot.windows.tools.64.exe!ProgressDialog::task_step(const String & p_task, const String & p_state, int p_step, bool p_force_redraw) Line 211	C++
 	godot.windows.tools.64.exe!EditorNode::progress_task_step(const String & p_task, const String & p_state, int p_step, bool p_force_refresh) Line 3958	C++
 	godot.windows.tools.64.exe!EditorProgress::step(const String & p_state, int p_step, bool p_force_refresh) Line 886	C++
 	godot.windows.tools.64.exe!EditorFileSystem::reimport_files(const Vector<String> & p_files) Line 1940	C++
 	godot.windows.tools.64.exe!EditorFileSystem::_update_scan_actions() Line 578	C++
 	godot.windows.tools.64.exe!EditorFileSystem::_notification(int p_what) Line 1137	C++
 	godot.windows.tools.64.exe!EditorFileSystem::_notificationv(int p_notification, bool p_reversed) Line 108	C++
 	godot.windows.tools.64.exe!Object::notification(int p_notification, bool p_reversed) Line 795	C++
 	godot.windows.tools.64.exe!SceneTree::_notify_group_pause(const StringName & p_group, int p_notification) Line 810	C++
 	godot.windows.tools.64.exe!SceneTree::process(float p_time) Line 442	C++
 	godot.windows.tools.64.exe!Main::iteration() Line 2489	C++
 	godot.windows.tools.64.exe!OS_Windows::run() Line 622	C++
 	godot.windows.tools.64.exe!widechar_main(int argc, wchar_t * * argv) Line 163	C++
 	godot.windows.tools.64.exe!_main() Line 185	C++
 	godot.windows.tools.64.exe!main(int argc, char * * argv) Line 199	C++

I find the code at ProcessDialog::task_step particularly concerning. Performing a re-entrant call into Main::iteration is not something to be taken lightly. See this comment:

	Main::iteration(); // this will not work on a lot of platforms, so it's only meant for the editor

There are a few concerning, related issues here:

  • I ran into unpredictable behavior when using threads.
  • Even when single-threaded, I ran into issues when calling ::scan after getting scan complete signal, even from call_deferred.
  • scan() appears to notify that it is complete (is_scanning becomes false), before the scanned assets are imported. Is this related? should it be its own issue?

Steps to reproduce:

I have made a backup both of my project and the version of godot which causes the issue.

It is reproducible within my project if I run my tool script, but it contains assets which cannot be redistributed. I have not sufficiently narrowed down the cause.

Minimal reproduction project:
None yet, sorry.

@lyuma
Copy link
Contributor Author

lyuma commented Apr 28, 2021

This is still a problem on the latest master ( 4.0.dev 1c2766e ):

CrashHandlerException: Program crashed
Dumping the backtrace. Please include this when reporting the bug on https://github.com/godotengine/godot/issues
[0] String::operator== (S:\repo\godot-pr-pr\core\string\ustring.cpp:580)
[1] String::operator== (S:\repo\godot-pr-pr\core\string\ustring.cpp:580)
[2] EditorFileSystemDirectory::find_file_index (S:\repo\godot-pr-pr\editor\editor_file_system.cpp:54)
[3] EditorFileSystem::_update_scan_actions (S:\repo\godot-pr-pr\editor\editor_file_system.cpp:551)
[4] EditorFileSystem::_notification (S:\repo\godot-pr-pr\editor\editor_file_system.cpp:1141)
[5] EditorFileSystem::_notificationv (S:\repo\godot-pr-pr\editor\editor_file_system.h:110)
[6] Object::notification (S:\repo\godot-pr-pr\core\object\object.cpp:795)
[7] SceneTree::_notify_group_pause (S:\repo\godot-pr-pr\scene\main\scene_tree.cpp:796)
[8] SceneTree::process (S:\repo\godot-pr-pr\scene\main\scene_tree.cpp:443)
[9] Main::iteration (S:\repo\godot-pr-pr\main\main.cpp:2505)
[10] ProgressDialog::task_step (S:\repo\godot-pr-pr\editor\progress_dialog.cpp:211)
[11] EditorNode::progress_task_step (S:\repo\godot-pr-pr\editor\editor_node.cpp:3987)
[12] EditorFileSystem::reimport_files (S:\repo\godot-pr-pr\editor\editor_file_system.cpp:2020)
[13] EditorFileSystem::_update_scan_actions (S:\repo\godot-pr-pr\editor\editor_file_system.cpp:582)
[14] EditorFileSystem::_notification (S:\repo\godot-pr-pr\editor\editor_file_system.cpp:1141)
[15] EditorFileSystem::_notificationv (S:\repo\godot-pr-pr\editor\editor_file_system.h:110)
[16] Object::notification (S:\repo\godot-pr-pr\core\object\object.cpp:795)
[17] SceneTree::_notify_group_pause (S:\repo\godot-pr-pr\scene\main\scene_tree.cpp:796)
[18] SceneTree::process (S:\repo\godot-pr-pr\scene\main\scene_tree.cpp:443)
[19] Main::iteration (S:\repo\godot-pr-pr\main\main.cpp:2505)
[20] ProgressDialog::task_step (S:\repo\godot-pr-pr\editor\progress_dialog.cpp:211)
[21] EditorNode::progress_task_step (S:\repo\godot-pr-pr\editor\editor_node.cpp:3987)
[22] EditorFileSystem::reimport_files (S:\repo\godot-pr-pr\editor\editor_file_system.cpp:2020)
[23] EditorFileSystem::_update_scan_actions (S:\repo\godot-pr-pr\editor\editor_file_system.cpp:582)
[24] EditorFileSystem::_notification (S:\repo\godot-pr-pr\editor\editor_file_system.cpp:1124)
[25] EditorFileSystem::_notificationv (S:\repo\godot-pr-pr\editor\editor_file_system.h:110)
[26] Object::notification (S:\repo\godot-pr-pr\core\object\object.cpp:795)
[27] SceneTree::_notify_group_pause (S:\repo\godot-pr-pr\scene\main\scene_tree.cpp:796)
[28] SceneTree::process (S:\repo\godot-pr-pr\scene\main\scene_tree.cpp:443)
[29] Main::iteration (S:\repo\godot-pr-pr\main\main.cpp:2505)
[30] ProgressDialog::task_step (S:\repo\godot-pr-pr\editor\progress_dialog.cpp:211)
[31] EditorNode::progress_task_step (S:\repo\godot-pr-pr\editor\editor_node.cpp:3987)
[32] EditorFileSystem::reimport_files (S:\repo\godot-pr-pr\editor\editor_file_system.cpp:2020)
[33] EditorFileSystem::_update_scan_actions (S:\repo\godot-pr-pr\editor\editor_file_system.cpp:582)
[34] EditorFileSystem::_notification (S:\repo\godot-pr-pr\editor\editor_file_system.cpp:1124)
[35] EditorFileSystem::_notificationv (S:\repo\godot-pr-pr\editor\editor_file_system.h:110)
[36] Object::notification (S:\repo\godot-pr-pr\core\object\object.cpp:795)
[37] SceneTree::_notify_group_pause (S:\repo\godot-pr-pr\scene\main\scene_tree.cpp:796)
[38] SceneTree::process (S:\repo\godot-pr-pr\scene\main\scene_tree.cpp:443)
[39] Main::iteration (S:\repo\godot-pr-pr\main\main.cpp:2505)
[40] ProgressDialog::task_step (S:\repo\godot-pr-pr\editor\progress_dialog.cpp:211)
[41] EditorNode::progress_task_step (S:\repo\godot-pr-pr\editor\editor_node.cpp:3987)
[42] ResourceImporterScene::import (S:\repo\godot-pr-pr\editor\import\resource_importer_scene.cpp:1327)
[43] EditorFileSystem::_reimport_file (S:\repo\godot-pr-pr\editor\editor_file_system.cpp:1779)
[44] EditorFileSystem::reimport_files (S:\repo\godot-pr-pr\editor\editor_file_system.cpp:2021)
[45] EditorFileSystem::_update_scan_actions (S:\repo\godot-pr-pr\editor\editor_file_system.cpp:582)
[46] EditorFileSystem::_notification (S:\repo\godot-pr-pr\editor\editor_file_system.cpp:1141)
[47] EditorFileSystem::_notificationv (S:\repo\godot-pr-pr\editor\editor_file_system.h:110)
[48] Object::notification (S:\repo\godot-pr-pr\core\object\object.cpp:795)
[49] SceneTree::_notify_group_pause (S:\repo\godot-pr-pr\scene\main\scene_tree.cpp:796)
[50] SceneTree::process (S:\repo\godot-pr-pr\scene\main\scene_tree.cpp:443)
[51] Main::iteration (S:\repo\godot-pr-pr\main\main.cpp:2505)
[52] OS_Windows::run (S:\repo\godot-pr-pr\platform\windows\os_windows.cpp:622)
[53] widechar_main (S:\repo\godot-pr-pr\platform\windows\godot_windows.cpp:163)
[54] _main (S:\repo\godot-pr-pr\platform\windows\godot_windows.cpp:187)
[55] main (S:\repo\godot-pr-pr\platform\windows\godot_windows.cpp:199)
[56] __scrt_common_main_seh (D:\agent\_work\9\s\src\vctools\crt\vcstartup\src\startup\exe_common.inl:288)
[57] BaseThreadInitThunk
-- END OF BACKTRACE --

@lyuma
Copy link
Contributor Author

lyuma commented Aug 25, 2022

Duplicate of #54864

@lyuma lyuma marked this as a duplicate of #54864 Aug 25, 2022
@lyuma lyuma closed this as completed Aug 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants