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

Assert in ClassDB::set_current_api in latest master when launched w/ --verbose #80062

Closed
darksylinc opened this issue Jul 30, 2023 · 12 comments · Fixed by #80089
Closed

Assert in ClassDB::set_current_api in latest master when launched w/ --verbose #80062

darksylinc opened this issue Jul 30, 2023 · 12 comments · Fixed by #80089

Comments

@darksylinc
Copy link
Contributor

darksylinc commented Jul 30, 2023

Godot version

master [75f9c97]

System information

Godot v4.2.dev (262d1ea) - Ubuntu 20.04.6 LTS (Focal Fossa) - X11 - Vulkan (Forward+) - dedicated AMD Radeon RX 6800 XT - AMD Ryzen 9 5900X 12-Core Processor (24 Threads)

Issue description

Running a dev build on both Linux and Windows reaches the following crash:

TextServer: Primary interface set to: "ICU / HarfBuzz / Graphite (Built-in)".
CORE API HASH: 2795494918
EDITOR API HASH: 1481585570
ERROR: FATAL: DEV_ASSERT failed  "!api_hashes_cache.has(p_api)" is false.
   at: set_current_api (core/object/class_db.cpp:59)

Call stack (Linux):

1 ClassDB::set_current_api                class_db.cpp        59   0x9c77bf6 
2 EditorSettings::ensure_class_registered editor_settings.cpp 883  0x639a96a 
3 EditorNode::EditorNode                  editor_node.cpp     6825 0x61badd9 
4 Main::start                             main.cpp            3085 0x4a6e353 
5 main                                    godot_linuxbsd.cpp  72   0x49c9d0d 

Call stack (Windows):

>	godot.windows.editor.dev.x86_64.exe!ClassDB::set_current_api(ClassDB::APIType p_api) Line 58	C++
 	godot.windows.editor.dev.x86_64.exe!EditorSettings::ensure_class_registered() Line 886	C++
 	godot.windows.editor.dev.x86_64.exe!EditorNode::EditorNode() Line 6826	C++
 	godot.windows.editor.dev.x86_64.exe!Main::start() Line 3085	C++
 	godot.windows.editor.dev.x86_64.exe!widechar_main(int argc, wchar_t * * argv) Line 179	C++
 	godot.windows.editor.dev.x86_64.exe!_main() Line 204	C++
 	godot.windows.editor.dev.x86_64.exe!main(int argc, char * * argv) Line 218	C++
 	godot.windows.editor.dev.x86_64.exe!WinMain(HINSTANCE__ * hInstance, HINSTANCE__ * hPrevInstance, char * lpCmdLine, int nCmdShow) Line 232	C++
 	[Inline Frame] godot.windows.editor.dev.x86_64.exe!invoke_main() Line 102	C++

Steps to reproduce

Compile with (possibly not all of those flags are needed):

scons platform=linuxbsd optimize=none debug_symbols=True tests=False dev_mode=True dev_build=True use_llvm=yes use_lld=yes opengl3=no openxr=no

Open a project on the editor with:

Working folder: The project's path
Arguments: -e --verbose

The --verbose argument is important!

Other info (for bisect)

It must've broken somewhere between 75f9c97 and 6588a4a since it was working fine before that.

Update: It's possible I'm wrong about the bisecting info, because the problem started when I added --verbose to the cmd line.

Workaround

Just commenting out the assert runs fine.

Minimal reproduction project

Any.

@AThousandShips
Copy link
Member

Cannot replicate on latest master, on Windows

@darksylinc
Copy link
Contributor Author

I was ignoring the error because I thought I may have done something wrong but then I got the same error today on Windows which is why I decided to report it.

Do you know what scons settings did you use? I'm uploading my .scons_env.json for Windows if that helps (I use vsproj=yes and build from Visual Studio which invokes scons).

scons_env.json.zip

@AThousandShips
Copy link
Member

AThousandShips commented Jul 30, 2023

I use the same settings as your Linux build minus the llvm, not via VS

@darksylinc
Copy link
Contributor Author

AHHH!!!!

Turns out I was missing a command line parameter: It must be launched with --verbose

@darksylinc darksylinc changed the title Assert in ClassDB::set_current_api in latest master Assert in ClassDB::set_current_api in latest master when launched w/ --verbose Jul 30, 2023
@clayjohn
Copy link
Member

I can confirm this issue, bisecting now

@AThousandShips
Copy link
Member

AThousandShips commented Jul 31, 2023

Is indeed caused by the --verbose flag as it causes:

godot/main/main.cpp

Lines 2583 to 2584 in 3fa8fad

print_verbose("CORE API HASH: " + uitos(ClassDB::get_api_hash(ClassDB::API_CORE)));
print_verbose("EDITOR API HASH: " + uitos(ClassDB::get_api_hash(ClassDB::API_EDITOR)));

And you cannot update the API hash if it has been read, as per here:

void ClassDB::set_current_api(APIType p_api) {
DEV_ASSERT(!api_hashes_cache.has(p_api)); // This API type may not be suitable for caching of hash if it can change later.
current_api = p_api;
}

The second part was done in #76467

But does seem to be a regression from #78615, as removing any reference to ensure_class_registered removes the crash, so seems there needs to be some better handling of the registering of things before the API hash is accessed

@clayjohn
Copy link
Member

I can confirm, this is a regression from #78615. Reverting #78615 fixes the crash

Thanks AThousandShips for finding that, you saved me a lot of bisecting time

@AThousandShips
Copy link
Member

AThousandShips commented Jul 31, 2023

I'm not sure why EditorSettings is not registered in register_editor_types, but registering it there seems to ensure it is registered, and no other errors popping up

So unless there's some specific reason EditorSettings shouldn't be registered like everything else that would fix this, along with removing the relevant method in EditorSettings

@clayjohn
Copy link
Member

CC @RandomShaper

@AThousandShips
Copy link
Member

AThousandShips commented Jul 31, 2023

Oh yes EditorSettings is already registered, as an abstract class, and then registered again as a non-abstract class, unsure why, only found that now, that is strange

Edit: It was made virtual all the way back in #12796, however EditorSettings can still be constructed in GDScript, as it is later registered as a non-abstract class, so this doesn't really apply

Edit 2: With some tweaks to documentation generation the EditorSettings documentation works correctly, and will test if some other areas cause problems

Will open a PR in a bit if this is considered a realistic solution

@RandomShaper
Copy link
Member

For the records, I put that DEV_ASSERT there precisely to realize when the caching of hashes wouldn't be reliable. I didn't expect that to happen that soon, though. In any case, the problem is that the editor API is modified after its hash having already been computed, which was an issue in itself.

@AThousandShips
Copy link
Member

I think that check is appropriate there

@akien-mga akien-mga added this to the 4.2 milestone Aug 2, 2023
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.

5 participants