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

Godot crash when printing info about leaked nodes due to optional forced flush #44850

Closed
qarmin opened this issue Jan 1, 2021 · 3 comments · Fixed by #45853
Closed

Godot crash when printing info about leaked nodes due to optional forced flush #44850

qarmin opened this issue Jan 1, 2021 · 3 comments · Fixed by #45853

Comments

@qarmin
Copy link
Contributor

qarmin commented Jan 1, 2021

Godot version:
3.2.4.beta.custom_build. adfc646 - but code is similar in version 4

Regression from #44393 - CC @Calinou

OS/device including version:
Ubuntu 20.04

Issue description:
When info about leaked nodes is printed, then Godot crash

ERROR: ~List: Condition "_first != __null" is true.
   At: ./core/self_list.h:112.
ERROR: ~List: Condition "_first != __null" is true.
   At: ./core/self_list.h:112.
WARNING: cleanup: ObjectDB instances leaked at exit (run with --verbose for details).
   At: core/object.cpp:2132.
Leaked instance: GDScriptNativeClass:563
core/io/logger.cpp:234:7: runtime error: member access within null pointer of type 'struct ProjectSettings'
core/object.cpp:484:6: runtime error: member access within null pointer of type 'const struct Object'
handle_crash: Program crashed with signal 11
Dumping the backtrace. 
[1] godots() [0x13aec6a] (/mnt/Miecz/godot3.2/platform/x11/crash_handler_x11.cpp:54)
[2] /lib/x86_64-linux-gnu/libc.so.6(+0x46210) [0x7fc0f58c6210] (??:0)
[3] Object::get(StringName const&, bool*) const (/mnt/Miecz/godot3.2/core/object.cpp:484)
[4] StdLogger::logv(char const*, __va_list_tag*, bool) (/mnt/Miecz/godot3.2/core/io/logger.cpp:234 (discriminator 2))
[5] CompositeLogger::logv(char const*, __va_list_tag*, bool) (/mnt/Miecz/godot3.2/core/io/logger.cpp:256 (discriminator 1))
[6] OS::print(char const*, ...) (/mnt/Miecz/godot3.2/core/os/os.cpp:132)
[7] print_line(String) (/mnt/Miecz/godot3.2/core/print_string.cpp:80 (discriminator 4))
[8] ObjectDB::cleanup() (/mnt/Miecz/godot3.2/core/object.cpp:2149 (discriminator 17))
[9] unregister_core_types() (/mnt/Miecz/godot3.2/core/register_core_types.cpp:313)
[10] Main::cleanup() (/mnt/Miecz/godot3.2/main/main.cpp:2299)
[11] godots(main+0x336) [0x13a5b5c] (/mnt/Miecz/godot3.2/platform/x11/godot_x11.cpp:59)
[12] /lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf3) [0x7fc0f58a70b3] (??:0)
[13] godots(_start+0x2e) [0x13a576e] (??:?)
-- END OF BACKTRACE --
Przerwane (zrzut pamięci)

Relevant code - https://github.com/godotengine/godot/blame/adfc646f8cb497d520ea2238482ab674fe90d43a/core/io/logger.cpp#L234-L238

Steps to reproduce:

  1. Run minimal project with this flags - godot -e -q -v

Minimal reproduction project:
https://github.com/jegor377/godot-gdgifexporter/archive/8e754c09dc10ec1f25ec2510c688bc1b35ee6b64.zip

@akien-mga
Copy link
Member

@Calinou GLOBAL_GET is likely not safe to use in such core code which outlives ProjectSettings.
You should likely get the result in the constructor and cache it, not query in logv.

@akien-mga
Copy link
Member

akien-mga commented Jan 5, 2021

You should likely get the result in the constructor and cache it, not query in logv.

For the reference, GLOBAL_GET in the constructor segfaults, that's likely too early. We can't make assumptions on the available of ProjectSettings in the logging code which starts before most things and ends before most things.

It might also be possible to trigger a crash by printing stuff before the ProjectSettings are created.

If this should be made configurable, the option probably needs to be enabled in the ProjectSettings code.

I'll revert the 3.2 cherry-pick for now while we work on a solution for master.

@akien-mga akien-mga changed the title Godot crash when printing info about leaked nodes Godot crash when printing info about leaked nodes due to optional forced flush Jan 5, 2021
akien-mga added a commit to akien-mga/godot that referenced this issue Jan 5, 2021
…ilds"

This reverts commit 341b9cf.

This makes the logger crash when used during cleanup: godotengine#44850.
@groud
Copy link
Member

groud commented Feb 9, 2021

Would be great to fix it I need it for debugging. I'll try to fix it.

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