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

Orphan StringNames when closing project manager or empty project #92726

Open
akien-mga opened this issue Jun 3, 2024 · 6 comments · May be fixed by #99931
Open

Orphan StringNames when closing project manager or empty project #92726

akien-mga opened this issue Jun 3, 2024 · 6 comments · May be fixed by #99931

Comments

@akien-mga
Copy link
Member

Tested versions

  • Reproducible in 4.3.beta (5f1184e) and 4.3.beta1 (174 orphans)
  • Partially reproducible in 4.3.dev4 and 4.3.dev5 (3 orphans)
  • Partially reproducible in 4.3.dev6 (9 orphans)

System information

Fedora Linux 40 (KDE Plasma) - Wayland - Vulkan (Forward+) - dedicated AMD Radeon RX 7600M XT (RADV NAVI33) () - AMD Ryzen 7 7840HS w/ Radeon 780M Graphics (16 Threads)

Issue description

When closing the project manager or the editor with --verbose mode, I get 174 orphan StringNames reported:

Orphan StringName: greaterThanEqual (static: 0, total: 1)
Orphan StringName: packSnorm4x8 (static: 0, total: 1)
Orphan StringName: textureGrad (static: 0, total: 1)
Orphan StringName: uaddCarry (static: 0, total: 1)
Orphan StringName: InputMap (static: 1, total: 3)
Orphan StringName: frexp (static: 0, total: 1)
Orphan StringName: uint (static: 0, total: 1)
Orphan StringName: bitfieldInsert (static: 0, total: 1)
Orphan StringName: GDExtensionManager (static: 1, total: 3)
Orphan StringName: packSnorm2x16 (static: 0, total: 1)
Orphan StringName: sqrt (static: 0, total: 1)
Orphan StringName: normalize (static: 0, total: 1)
Orphan StringName: notEqual (static: 0, total: 1)
Orphan StringName: usubBorrow (static: 0, total: 1)
Orphan StringName: ceil (static: 0, total: 1)
Orphan StringName: TextServerManager (static: 1, total: 4)
Orphan StringName: step (static: 0, total: 1)
Orphan StringName: dFdx (static: 0, total: 1)
Orphan StringName: dFdy (static: 0, total: 1)
Orphan StringName: asinh (static: 0, total: 1)
Orphan StringName: findLSB (static: 0, total: 1)
Orphan StringName: lessThan (static: 0, total: 1)
Orphan StringName: findMSB (static: 0, total: 1)
Orphan StringName: texelFetch (static: 0, total: 1)
Orphan StringName: log2 (static: 0, total: 1)
Orphan StringName: WorkerThreadPool (static: 1, total: 3)
Orphan StringName: ThemeDB (static: 1, total: 3)
Orphan StringName: imulExtended (static: 0, total: 1)
Orphan StringName: asin (static: 0, total: 1)
Orphan StringName: textureQueryLod (static: 0, total: 1)
Orphan StringName: Input (static: 1, total: 3)
Orphan StringName: atan (static: 0, total: 1)
Orphan StringName: smoothstep (static: 0, total: 1)
Orphan StringName: PhysicsServer3DManager (static: 1, total: 4)
Orphan StringName: cosh (static: 0, total: 1)
Orphan StringName: PhysicsServer2D (static: 1, total: 4)
Orphan StringName: PhysicsServer3D (static: 1, total: 4)
Orphan StringName: ResourceLoader (static: 1, total: 3)
Orphan StringName: texture (static: 0, total: 1)
Orphan StringName: floatBitsToInt (static: 0, total: 1)
Orphan StringName: XRServer (static: 1, total: 4)
Orphan StringName: AudioServer (static: 1, total: 4)
Orphan StringName: unpackUnorm4x8 (static: 0, total: 1)
Orphan StringName: ResourceUID (static: 1, total: 3)
Orphan StringName: faceforward (static: 0, total: 1)
Orphan StringName: tanh (static: 0, total: 1)
Orphan StringName: transpose (static: 0, total: 1)
Orphan StringName: abs (static: 0, total: 1)
Orphan StringName: all (static: 0, total: 1)
Orphan StringName: clamp (static: 0, total: 1)
Orphan StringName: any (static: 0, total: 1)
Orphan StringName: bitCount (static: 0, total: 1)
Orphan StringName: cos (static: 0, total: 1)
Orphan StringName: lessThanEqual (static: 0, total: 1)
Orphan StringName: reflect (static: 0, total: 1)
Orphan StringName: RenderingServer (static: 1, total: 4)
Orphan StringName: dot (static: 0, total: 1)
Orphan StringName: EditorFileDialog (static: 1, total: 8)
Orphan StringName: determinant (static: 0, total: 1)
Orphan StringName: exp (static: 0, total: 1)
Orphan StringName: fwidthCoarse (static: 0, total: 1)
Orphan StringName: NavigationServer2D (static: 0, total: 3)
Orphan StringName: NavigationServer3D (static: 1, total: 4)
Orphan StringName: CameraServer (static: 1, total: 4)
Orphan StringName: AudioStream (static: 0, total: 1)
Orphan StringName: textureSize (static: 0, total: 1)
Orphan StringName: fma (static: 0, total: 1)
Orphan StringName: IP (static: 0, total: 3)
Orphan StringName: OS (static: 1, total: 3)
Orphan StringName: ItemList (static: 1, total: 10)
Orphan StringName: intBitsToFloat (static: 0, total: 1)
Orphan StringName: acosh (static: 0, total: 1)
Orphan StringName: degrees (static: 0, total: 1)
Orphan StringName: EditorInterface (static: 1, total: 3)
Orphan StringName: ivec2 (static: 0, total: 1)
Orphan StringName: ivec3 (static: 0, total: 1)
Orphan StringName: ivec4 (static: 0, total: 1)
Orphan StringName: bitfieldExtract (static: 0, total: 1)
Orphan StringName: DisplayServer (static: 1, total: 4)
Orphan StringName: Marshalls (static: 1, total: 3)
Orphan StringName: vec2 (static: 0, total: 1)
Orphan StringName: vec3 (static: 0, total: 1)
Orphan StringName: vec4 (static: 0, total: 1)
Orphan StringName: exp2 (static: 0, total: 1)
Orphan StringName: dFdxCoarse (static: 0, total: 1)
Orphan StringName: int (static: 0, total: 1)
Orphan StringName: mat2 (static: 0, total: 1)
Orphan StringName: mat3 (static: 0, total: 1)
Orphan StringName: mat4 (static: 0, total: 1)
Orphan StringName: unpackHalf2x16 (static: 0, total: 1)
Orphan StringName: TabBar (static: 1, total: 10)
Orphan StringName: textureProjLod (static: 0, total: 1)
Orphan StringName: inversesqrt (static: 0, total: 1)
Orphan StringName: log (static: 0, total: 1)
Orphan StringName: Geometry2D (static: 1, total: 3)
Orphan StringName: Geometry3D (static: 1, total: 3)
Orphan StringName: textureGather (static: 0, total: 1)
Orphan StringName: max (static: 0, total: 1)
Orphan StringName: atanh (static: 0, total: 1)
Orphan StringName: min (static: 0, total: 1)
Orphan StringName: mix (static: 0, total: 1)
Orphan StringName: mod (static: 0, total: 1)
Orphan StringName: FileDialog (static: 1, total: 8)
Orphan StringName: not (static: 0, total: 1)
Orphan StringName: umulExtended (static: 0, total: 1)
Orphan StringName: AudioStreamRandomizer (static: 0, total: 5)
Orphan StringName: dFdxFine (static: 0, total: 1)
Orphan StringName: MenuButton (static: 1, total: 2)
Orphan StringName: trunc (static: 0, total: 1)
Orphan StringName: pow (static: 0, total: 1)
Orphan StringName: inverse (static: 0, total: 1)
Orphan StringName: equal (static: 0, total: 1)
Orphan StringName: unpackSnorm4x8 (static: 0, total: 1)
Orphan StringName: uvec2 (static: 0, total: 1)
Orphan StringName: uvec3 (static: 0, total: 1)
Orphan StringName: uvec4 (static: 0, total: 1)
Orphan StringName: refract (static: 0, total: 1)
Orphan StringName: NativeMenu (static: 1, total: 4)
Orphan StringName: sin (static: 0, total: 1)
Orphan StringName: packUnorm4x8 (static: 0, total: 1)
Orphan StringName: OptionButton (static: 1, total: 12)
Orphan StringName: tan (static: 0, total: 1)
Orphan StringName: ResourceSaver (static: 1, total: 3)
Orphan StringName: dFdyFine (static: 0, total: 1)
Orphan StringName: bool (static: 0, total: 1)
Orphan StringName: textureProjGrad (static: 0, total: 1)
Orphan StringName: unpackUnorm2x16 (static: 0, total: 1)
Orphan StringName: textureLod (static: 0, total: 1)
Orphan StringName: modf (static: 0, total: 1)
Orphan StringName: dFdyCoarse (static: 0, total: 1)
Orphan StringName: ProjectSettings (static: 1, total: 3)
Orphan StringName: floatBitsToUint (static: 0, total: 1)
Orphan StringName: Performance (static: 1, total: 3)
Orphan StringName: outerProduct (static: 0, total: 1)
Orphan StringName: Engine (static: 1, total: 3)
Orphan StringName: round (static: 0, total: 1)
Orphan StringName: distance (static: 0, total: 1)
Orphan StringName: PhysicsServer2DManager (static: 1, total: 4)
Orphan StringName: textureQueryLevels (static: 0, total: 1)
Orphan StringName: roundEven (static: 0, total: 1)
Orphan StringName: bvec2 (static: 0, total: 1)
Orphan StringName: bvec3 (static: 0, total: 1)
Orphan StringName: bvec4 (static: 0, total: 1)
Orphan StringName: fwidthFine (static: 0, total: 1)
Orphan StringName: unpackSnorm2x16 (static: 0, total: 1)
Orphan StringName: JavaClassWrapper (static: 1, total: 3)
Orphan StringName: uintBitsToFloat (static: 0, total: 1)
Orphan StringName: float (static: 0, total: 1)
Orphan StringName: radians (static: 0, total: 1)
Orphan StringName: JavaScriptBridge (static: 1, total: 3)
Orphan StringName: floor (static: 0, total: 1)
Orphan StringName: packHalf2x16 (static: 0, total: 1)
Orphan StringName: greaterThan (static: 0, total: 1)
Orphan StringName: cross (static: 0, total: 1)
Orphan StringName: fwidth (static: 0, total: 1)
Orphan StringName: bitfieldReverse (static: 0, total: 1)
Orphan StringName: Texture2D (static: 0, total: 5)
Orphan StringName: isinf (static: 0, total: 1)
Orphan StringName: ldexp (static: 0, total: 1)
Orphan StringName: NavigationMeshGenerator (static: 1, total: 3)
Orphan StringName: length (static: 0, total: 1)
Orphan StringName: sign (static: 0, total: 1)
Orphan StringName: sinh (static: 0, total: 1)
Orphan StringName: Time (static: 1, total: 3)
Orphan StringName: packUnorm2x16 (static: 0, total: 1)
Orphan StringName: matrixCompMult (static: 0, total: 1)
Orphan StringName: fract (static: 0, total: 1)
Orphan StringName: ClassDB (static: 1, total: 3)
Orphan StringName: textureProj (static: 0, total: 1)
Orphan StringName: acos (static: 0, total: 1)
Orphan StringName: EngineDebugger (static: 1, total: 3)
Orphan StringName: TranslationServer (static: 1, total: 3)
Orphan StringName: PopupMenu (static: 1, total: 16)
Orphan StringName: isnan (static: 0, total: 1)
StringName: 174 unclaimed string names at exit.

Most of this seems to have been introduced between dev 6 and beta 1.

In previous snapshots, there's a subset of those that were reported:

In 4.3.dev6:

Orphan StringName: EditorFileDialog (static: 1, total: 7)
Orphan StringName: AudioStream (static: 0, total: 1)
Orphan StringName: ItemList (static: 1, total: 9)
Orphan StringName: TabBar (static: 1, total: 9)
Orphan StringName: FileDialog (static: 1, total: 7)
Orphan StringName: AudioStreamRandomizer (static: 0, total: 4)
Orphan StringName: OptionButton (static: 1, total: 11)
Orphan StringName: Texture2D (static: 0, total: 5)
Orphan StringName: PopupMenu (static: 1, total: 15)
StringName: 9 unclaimed string names at exit.

And in 4.3.dev4 / 4.3.dev5:

Orphan StringName: ItemList (static: 1, total: 9)
Orphan StringName: Texture2D (static: 0, total: 2)
Orphan StringName: PopupMenu (static: 1, total: 15)
StringName: 3 unclaimed string names at exit.

Steps to reproduce

  • godot -v, close it and check terminal

Minimal reproduction project (MRP)

n/a

@akien-mga akien-mga added this to the 4.3 milestone Jun 3, 2024
@akien-mga
Copy link
Member Author

akien-mga commented Jun 3, 2024

I bisected the dev4 regression to #88162, CC @KoBeWi.

The dev6 regression is from #88306, which adds more PropertyListHelper uses.
The EditorFileDialog orphan seems to be a bit older than #88306, I didn't bisect it separately.

Now bisecting the beta1 part.

@KoBeWi
Copy link
Member

KoBeWi commented Jun 3, 2024

Sounds like another case of "leaks" reported as leaks, because they get freed too late. Each class that uses PropertyListHelper has a static instance which gets freed only once the program fully exits, which is likely after the leak detector. The only solution would be manually freeing static helpers, not sure where though.

@akien-mga
Copy link
Member Author

akien-mga commented Jun 3, 2024

Now bisecting the beta1 part.

Some of the beta1 "leaks" seem to be Engine singletons, I suspect this was caused by #92060, CC @raulsntos.

The last chunk of "leaks" seems to come from #92564, CC @Chaosus.
These ones are easily fixable with:

diff --git a/servers/rendering/shader_language.cpp b/servers/rendering/shader_language.cpp
index 9aa54d0bb7..ad7ed07b41 100644
--- a/servers/rendering/shader_language.cpp
+++ b/servers/rendering/shader_language.cpp
@@ -10732,4 +10732,5 @@ ShaderLanguage::ShaderLanguage() {
 
 ShaderLanguage::~ShaderLanguage() {
 	clear();
+	global_func_set.clear();
 }

(could also go in the clear() method, but it's called several times and I think set only needs to be populated in the constructor and removed in the destructor)

@Chaosus
Copy link
Member

Chaosus commented Jul 13, 2024

This one exist since dev1 build:

345328617-0dd325c0-7db1-4708-86fc-b2809739ed7a

If someone wants to help to hunt this bug down to close this issue. It's related with editor nodes since it doesn't happen at exit of Project Manager.

@KoBeWi KoBeWi removed this from the 4.3 milestone Jul 31, 2024
@vms-code
Copy link

vms-code commented Sep 5, 2024

@akien-mga @Chaosus @KoBeWi I solved this problem but I'm using a 4.3.dev version that is before the merge that solved the other orphan StringNames, for the other StringNames the issue was on PropertyInfo class_name, so I changed it to a String instead of StringName and it solved, I believe it was happening whenever a class used a static PropertyListHelper to register properties. For the remaining orphan StringName (Node) the problem comes from the following places:

// editor/editor_node.cpp  line 4570;
ClassDB::is_parent_class(p_class, SNAME("Node"))

// edior/scene_create_dialog.cpp line 55:
node_type_other->add_theme_icon_override(SNAME("icon"), get_editor_theme_icon(SNAME("Node")));

// editor/scene_tree_dock.cpp -> line 1506:
filter_menu->set_item_icon(filter_menu->get_item_index(FILTER_BY_TYPE), get_editor_theme_icon(SNAME("Node")));

// modules/multiplayer/editor/editor_network_profiler.cpp line 66:
theme_cache.node_icon = get_theme_icon(SNAME("Node"), EditorStringName(EditorIcons));

// editor/create_dialog.cpp line 505:
String text = help_bit->get_class_description(p_type);

for the places using SNAME, removing SNAME solves the issue (but not sure if it's a good solution), for the create_dialog.cpp line 505 situation I believe is happening because the StringName is placed on a static HashMap and the map is not cleaned, but I still haven't tried to solve yet...

I think the problem happens whenever StringName is initialized using static because it lives longer then the StringName::cleanup function call point (as it was suggested on @KoBeWi reply), but I still don't understand why it happens on the places using SNAME since there are several other places that also use SNAME and don't have a problem, the only explanation that makes sense to me now is that on this specific cases the StringName is also used somewhere else so it is referenced one extra time and because it is static as a byproduct of the SNAME macro the extra reference is not dereferenced correctly, does anyone know what is going on? Also, how does SNAME (static StringName) gets automatically dereferenced/deleted on other situations? Is it because of the SafeNumeric functionality? Would appreciate any insights or direction on resources to learn more about this... I'm going to spend more time trying to figure it out as well and maybe post an update if I believe I understood what is happening.

@vms-code
Copy link

I understand what is happening now, the problem is that the engine never cleans static StringNames, the reason why only those instances trigger the warning message is because of the condition:

if (d->static_count.get() != d->refcount.get())

and the situation that triggers the warning is the only one where there is a difference between refcount and static_count, by cleaning up the static HashMap on the situation where the StringName is not static the warning goes away:

~EditorHelpBit() {
    doc_class_cache.clear();
};

but this still doesn't clean the static StringNames, so I'm not sure about a good solution yet but one I'm trying to implement is to have a static Vector that keeps a pointer to all static StringNames, then loop over the vector and call unref on all of them before the logic that cleans the Data on the _table and displays the message, then change the condition to just check for:

if (d->refcount.get()) {
...
}

instead

@Chaosus Chaosus linked a pull request Dec 2, 2024 that will close this issue
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