From 582ed157b5f4ad73efc596f7e79b6c8778d3cbe1 Mon Sep 17 00:00:00 2001 From: Yuri Sizov Date: Wed, 18 Oct 2023 17:36:20 +0200 Subject: [PATCH] Fix StringName leaks in GDExtension, core, and editor themes --- core/core_constants.cpp | 2 ++ core/extension/gdextension.cpp | 6 ++++- core/extension/gdextension.h | 3 +++ core/extension/gdextension_compat_hashes.cpp | 4 ++++ core/extension/gdextension_compat_hashes.h | 1 + core/extension/gdextension_manager.cpp | 6 +++++ core/extension/gdextension_manager.h | 1 + core/register_core_types.cpp | 1 + editor/editor_node.cpp | 3 +++ editor/editor_themes.cpp | 25 +++++++++++++------- editor/editor_themes.h | 9 ++++--- editor/project_manager.cpp | 4 ++++ 12 files changed, 52 insertions(+), 13 deletions(-) diff --git a/core/core_constants.cpp b/core/core_constants.cpp index 33b32714957..2f70fdf2196 100644 --- a/core/core_constants.cpp +++ b/core/core_constants.cpp @@ -794,6 +794,8 @@ void register_global_constants() { void unregister_global_constants() { _global_constants.clear(); + _global_constants_map.clear(); + _global_enums.clear(); } int CoreConstants::get_global_constant_count() { diff --git a/core/extension/gdextension.cpp b/core/extension/gdextension.cpp index 8fb1aab6dcf..26512d0c562 100644 --- a/core/extension/gdextension.cpp +++ b/core/extension/gdextension.cpp @@ -664,7 +664,7 @@ void GDExtension::_get_library_path(GDExtensionClassLibraryPtr p_library, GDExte memnew_placement(r_path, String(self->library_path)); } -HashMap gdextension_interface_functions; +HashMap GDExtension::gdextension_interface_functions; void GDExtension::register_interface_function(StringName p_function_name, GDExtensionInterfaceFunctionPtr p_function_pointer) { ERR_FAIL_COND_MSG(gdextension_interface_functions.has(p_function_name), "Attempt to register interface function '" + p_function_name + "', which appears to be already registered."); @@ -836,6 +836,10 @@ void GDExtension::initialize_gdextensions() { register_interface_function("get_library_path", (GDExtensionInterfaceFunctionPtr)&GDExtension::_get_library_path); } +void GDExtension::finalize_gdextensions() { + gdextension_interface_functions.clear(); +} + Error GDExtensionResourceLoader::load_gdextension_resource(const String &p_path, Ref &p_extension) { ERR_FAIL_COND_V_MSG(p_extension.is_valid() && p_extension->is_library_open(), ERR_ALREADY_IN_USE, "Cannot load GDExtension resource into already opened library."); diff --git a/core/extension/gdextension.h b/core/extension/gdextension.h index d71b1f97044..bab3bcd1983 100644 --- a/core/extension/gdextension.h +++ b/core/extension/gdextension.h @@ -106,6 +106,8 @@ class GDExtension : public Resource { void clear_instance_bindings(); #endif + static HashMap gdextension_interface_functions; + protected: static void _bind_methods(); @@ -153,6 +155,7 @@ public: static void register_interface_function(StringName p_function_name, GDExtensionInterfaceFunctionPtr p_function_pointer); static GDExtensionInterfaceFunctionPtr get_interface_function(StringName p_function_name); static void initialize_gdextensions(); + static void finalize_gdextensions(); GDExtension(); ~GDExtension(); diff --git a/core/extension/gdextension_compat_hashes.cpp b/core/extension/gdextension_compat_hashes.cpp index 9c8d6b3e7fa..2dac4a3a5d8 100644 --- a/core/extension/gdextension_compat_hashes.cpp +++ b/core/extension/gdextension_compat_hashes.cpp @@ -840,4 +840,8 @@ void GDExtensionCompatHashes::initialize() { // clang-format on } +void GDExtensionCompatHashes::finalize() { + mappings.clear(); +} + #endif // DISABLE_DEPRECATED diff --git a/core/extension/gdextension_compat_hashes.h b/core/extension/gdextension_compat_hashes.h index 3a66ef0b976..29393dcb2d3 100644 --- a/core/extension/gdextension_compat_hashes.h +++ b/core/extension/gdextension_compat_hashes.h @@ -48,6 +48,7 @@ class GDExtensionCompatHashes { public: static void initialize(); + static void finalize(); static bool lookup_current_hash(const StringName &p_class, const StringName &p_method, uint32_t p_legacy_hash, uint32_t *r_current_hash); static bool get_legacy_hashes(const StringName &p_class, const StringName &p_method, Array &r_hashes); }; diff --git a/core/extension/gdextension_manager.cpp b/core/extension/gdextension_manager.cpp index 0dc84f685fa..a4d032f22f7 100644 --- a/core/extension/gdextension_manager.cpp +++ b/core/extension/gdextension_manager.cpp @@ -293,3 +293,9 @@ GDExtensionManager::GDExtensionManager() { GDExtensionCompatHashes::initialize(); #endif } + +GDExtensionManager::~GDExtensionManager() { +#ifndef DISABLE_DEPRECATED + GDExtensionCompatHashes::finalize(); +#endif +} diff --git a/core/extension/gdextension_manager.h b/core/extension/gdextension_manager.h index 8cd6d5a3e2a..9386e356bbe 100644 --- a/core/extension/gdextension_manager.h +++ b/core/extension/gdextension_manager.h @@ -86,6 +86,7 @@ public: void reload_extensions(); GDExtensionManager(); + ~GDExtensionManager(); }; VARIANT_ENUM_CAST(GDExtensionManager::LoadStatus) diff --git a/core/register_core_types.cpp b/core/register_core_types.cpp index b4ac5337798..4ad9dd43c48 100644 --- a/core/register_core_types.cpp +++ b/core/register_core_types.cpp @@ -360,6 +360,7 @@ void unregister_core_extensions() { if (_is_core_extensions_registered) { gdextension_manager->deinitialize_extensions(GDExtension::INITIALIZATION_LEVEL_CORE); } + GDExtension::finalize_gdextensions(); } void unregister_core_types() { diff --git a/editor/editor_node.cpp b/editor/editor_node.cpp index dc3afc2f1db..dcc9255dc6e 100644 --- a/editor/editor_node.cpp +++ b/editor/editor_node.cpp @@ -6951,6 +6951,7 @@ EditorNode::EditorNode() { // Exporters might need the theme. EditorColorMap::create(); + EditorTheme::initialize(); theme = create_custom_theme(); DisplayServer::set_early_window_clear_color_override(true, theme->get_color(SNAME("background"), EditorStringName(Editor))); @@ -8038,6 +8039,8 @@ EditorNode::~EditorNode() { memdelete(progress_hb); EditorSettings::destroy(); + EditorColorMap::finish(); + EditorTheme::finalize(); GDExtensionEditorPlugins::editor_node_add_plugin = nullptr; GDExtensionEditorPlugins::editor_node_remove_plugin = nullptr; diff --git a/editor/editor_themes.cpp b/editor/editor_themes.cpp index f5be91c6deb..bf0c5392c1a 100644 --- a/editor/editor_themes.cpp +++ b/editor/editor_themes.cpp @@ -55,7 +55,7 @@ void EditorColorMap::add_conversion_color_pair(const String p_from_color, const color_conversion_map[Color::html(p_from_color)] = Color::html(p_to_color); } -void EditorColorMap::add_conversion_exception(const StringName p_icon_name) { +void EditorColorMap::add_conversion_exception(const StringName &p_icon_name) { color_conversion_exceptions.insert(p_icon_name); } @@ -63,7 +63,7 @@ void EditorColorMap::create() { // Some of the colors below are listed for completeness sake. // This can be a basis for proper palette validation later. - // Convert: FROM TO + // Convert: FROM TO add_conversion_color_pair("#478cbf", "#478cbf"); // Godot Blue add_conversion_color_pair("#414042", "#414042"); // Godot Gray @@ -215,6 +215,11 @@ void EditorColorMap::create() { add_conversion_exception("Breakpoint"); } +void EditorColorMap::finish() { + color_conversion_map.clear(); + color_conversion_exceptions.clear(); +} + Vector EditorTheme::editor_theme_types; // TODO: Refactor these and corresponding Theme methods to use the bool get_xxx(r_value) pattern internally. @@ -301,13 +306,15 @@ Ref EditorTheme::get_stylebox(const StringName &p_name, const StringNa } } -EditorTheme::EditorTheme() { - if (editor_theme_types.is_empty()) { - editor_theme_types.append(EditorStringName(Editor)); - editor_theme_types.append(EditorStringName(EditorFonts)); - editor_theme_types.append(EditorStringName(EditorIcons)); - editor_theme_types.append(EditorStringName(EditorStyles)); - } +void EditorTheme::initialize() { + editor_theme_types.append(EditorStringName(Editor)); + editor_theme_types.append(EditorStringName(EditorFonts)); + editor_theme_types.append(EditorStringName(EditorIcons)); + editor_theme_types.append(EditorStringName(EditorStyles)); +} + +void EditorTheme::finalize() { + editor_theme_types.clear(); } // Editor theme generatior. diff --git a/editor/editor_themes.h b/editor/editor_themes.h index 7ca0050103a..7d913ccc408 100644 --- a/editor/editor_themes.h +++ b/editor/editor_themes.h @@ -45,12 +45,14 @@ class EditorColorMap { static HashSet color_conversion_exceptions; public: - static void create(); static void add_conversion_color_pair(const String p_from_color, const String p_to_color); - static void add_conversion_exception(const StringName p_icon_name); + static void add_conversion_exception(const StringName &p_icon_name); static HashMap &get_color_conversion_map() { return color_conversion_map; }; static HashSet &get_color_conversion_exceptions() { return color_conversion_exceptions; }; + + static void create(); + static void finish(); }; class EditorTheme : public Theme { @@ -66,7 +68,8 @@ public: virtual Ref get_icon(const StringName &p_name, const StringName &p_theme_type) const override; virtual Ref get_stylebox(const StringName &p_name, const StringName &p_theme_type) const override; - EditorTheme(); + static void initialize(); + static void finalize(); }; Ref create_editor_theme(Ref p_theme = nullptr); diff --git a/editor/project_manager.cpp b/editor/project_manager.cpp index c9555554e1e..de807c79e0a 100644 --- a/editor/project_manager.cpp +++ b/editor/project_manager.cpp @@ -2845,6 +2845,7 @@ ProjectManager::ProjectManager() { } EditorColorMap::create(); + EditorTheme::initialize(); Ref theme = create_custom_theme(); DisplayServer::set_early_window_clear_color_override(true, theme->get_color(SNAME("background"), EditorStringName(Editor))); @@ -3297,6 +3298,9 @@ ProjectManager::~ProjectManager() { if (EditorSettings::get_singleton()) { EditorSettings::destroy(); } + + EditorColorMap::finish(); + EditorTheme::finalize(); } void ProjectTag::_notification(int p_what) {