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

Off-thread access error spam in specific case from AnimationTree #102106

Closed
a-johnston opened this issue Jan 28, 2025 · 4 comments · Fixed by #102692
Closed

Off-thread access error spam in specific case from AnimationTree #102106

a-johnston opened this issue Jan 28, 2025 · 4 comments · Fixed by #102692

Comments

@a-johnston
Copy link
Contributor

a-johnston commented Jan 28, 2025

Tested versions

Reproducible on and after [53f3143] but I have no idea why this commit would be related. Bizarrely while initially bisecting I was only able to reproduce the error on mono builds and not standard, but after git clean -fdx and rebuilding to make sure I was only able to reproduce on standard and not mono.

System information

Godot v4.4.beta.mono (6dc78c8) - macOS Sequoia (15.2.0) - Multi-window, 1 monitor - Metal (Forward+) - integrated Apple M2 (Apple8) - Apple M2 (8 threads)

Issue description

This error gets printed out once per animation node within an animation tree (at least a blend tree) when opening the project and sometimes when running the project/opening a scene as well:

ERROR: This function in this node (/root/@EditorNode@21262/@Panel@14/@VBoxContainer@15/DockHSplitLeftL/DockHSplitLeftR/DockHSplitMain/@VBoxContainer@26/DockVSplitCenter/@EditorBottomPanel@7939/@VBoxContainer@7924/@AnimationTreeEditor@15287) can only be accessed from either the main thread or a thread group. Use call_deferred() instead.
   at: is_visible (scene/main/canvas_item.cpp:124)

at least in this case (I'm not sure if there are other triggering cases) is_visible is called by AnimationTreeEditor::get_animation_list. The error can be avoided by changing the condition in that method to:

if (!singleton->tree || !singleton->is_accessible_from_caller_thread() || !singleton->is_visible()) {
    [ .. ]
}

however this feels like a bad hack, especially without understanding the root cause

Steps to reproduce

In an affected version, create an AnimationTree node with a blend tree root and add animation playback nodes to the tree. There doesn't need to be an animation library or an animation set on the node for the error to print. Save the scene and close + reopen the editor.

Minimal reproduction project (MRP)

mrp.zip

@AThousandShips AThousandShips added this to the 4.4 milestone Jan 28, 2025
@akien-mga akien-mga changed the title [Godot 4.4] Off-thread access error spam in specific case from AnimationTree Off-thread access error spam in specific case from AnimationTree Feb 10, 2025
@akien-mga
Copy link
Member

I've tried to follow the steps to reproduce the issue but couldn't trigger it (on Linux, Fedora 41).

You mention a MRP is n/a, but the steps you outline seem like they'd give a reproducible project? Can you attach a MRP that reproduces it reliably for you? If it's platform/timing specific, that will also make it easier to see if others can trigger the bug.

@a-johnston
Copy link
Contributor Author

Sorry, I've updated the project with an mrp. The reason I was hesitant to include one was because although there needed to be an animation tree, or at least seemed to need, the project itself did not necessarily reproduce the issue. It seemed somehow dependent on compilation options when I was trying to repro/nail it down before.

With the current mrp on master (296de7d), I did git clean -fdx, scons, then when opening the project got this output:

Godot Engine v4.4.beta.custom_build.296de7da8 (2025-02-10 18:21:36 UTC) - https://godotengine.org
Metal 3.2 - Forward+ - Using Device #0: Apple - Apple M2 (Apple8)

2025-02-10 15:41:47.849 godot.macos.editor.arm64[47659:9504874] +[IMKClient subclass]: chose IMKClient_Modern
2025-02-10 15:41:47.849 godot.macos.editor.arm64[47659:9504874] +[IMKInputSession subclass]: chose IMKInputSession_Modern
ERROR: This function in this node (/root/@EditorNode@21278/@Panel@14/@VBoxContainer@15/DockHSplitLeftL/DockHSplitLeftR/DockHSplitMain/@VBoxContainer@26/DockVSplitCenter/@EditorBottomPanel@7939/@VBoxContainer@7924/@AnimationTreeEditor@15332) can only be accessed from either the main thread or a thread group. Use call_deferred() instead.
   at: is_visible (scene/main/canvas_item.cpp:124)
ERROR: This function in this node (/root/@EditorNode@21278/@Panel@14/@VBoxContainer@15/DockHSplitLeftL/DockHSplitLeftR/DockHSplitMain/@VBoxContainer@26/DockVSplitCenter/@EditorBottomPanel@7939/@VBoxContainer@7924/@AnimationTreeEditor@15332) can only be accessed from either the main thread or a thread group. Use call_deferred() instead.
   at: is_visible (scene/main/canvas_item.cpp:124)

On a larger project with many more animation nodes this turned into a few pages of console spam. I'll try to repro on windows/linux in a bit.

@a-johnston
Copy link
Contributor Author

a-johnston commented Feb 11, 2025

This seems maybe specific to macs. I was not able to reproduce on windows or linux with either the official 4.4.beta3 builds or my own off the current head but on macos I'm able to reproduce on both master and 4.4.beta3 even when I backup and remove Library/Application Support/Godot.

Edit: So strange. I remember double checking the bisect I did for the original issue but now it bisects to 2675997. Probably a red herring. total red herring

@a-johnston
Copy link
Contributor Author

Ok I could've found this much more easily lol. Lessons for next time I guess. This is being caused by EditorResourcePreview::_thread() in combination with AnimationNodeAnimation::_validate_property. On older commits such as e2c6daf, and also just sort of randomly on newer ones, get_animation_list is still called off the main thread but quits from !singleton->tree and does not trigger this error message. Potentially some change caused the tree to be assigned earlier than it used to be, or maybe it's just a race. I'm not sure why this wasn't reproducible on other platforms, I might go back and check a few things knowing the cause now.

Considering the error is printed when the preview is being generated and getting the animation list only serves to assign the property hint, I think it would be reasonable to just not add that hint if not on the main thread. The current behavior anyways gives back an empty list of options.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Not Critical
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants