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

Fix StringName leaks in GDExtension, core, and editor themes #83562

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions core/core_constants.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
6 changes: 5 additions & 1 deletion core/extension/gdextension.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -664,7 +664,7 @@ void GDExtension::_get_library_path(GDExtensionClassLibraryPtr p_library, GDExte
memnew_placement(r_path, String(self->library_path));
}

HashMap<StringName, GDExtensionInterfaceFunctionPtr> gdextension_interface_functions;
HashMap<StringName, GDExtensionInterfaceFunctionPtr> 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.");
Expand Down Expand Up @@ -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<GDExtension> &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.");

Expand Down
3 changes: 3 additions & 0 deletions core/extension/gdextension.h
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,8 @@ class GDExtension : public Resource {
void clear_instance_bindings();
#endif

static HashMap<StringName, GDExtensionInterfaceFunctionPtr> gdextension_interface_functions;

protected:
static void _bind_methods();

Expand Down Expand Up @@ -153,6 +155,7 @@ class GDExtension : public Resource {
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();
Expand Down
4 changes: 4 additions & 0 deletions core/extension/gdextension_compat_hashes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -840,4 +840,8 @@ void GDExtensionCompatHashes::initialize() {
// clang-format on
}

void GDExtensionCompatHashes::finalize() {
mappings.clear();
}

#endif // DISABLE_DEPRECATED
1 change: 1 addition & 0 deletions core/extension/gdextension_compat_hashes.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
};
Expand Down
6 changes: 6 additions & 0 deletions core/extension/gdextension_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -293,3 +293,9 @@ GDExtensionManager::GDExtensionManager() {
GDExtensionCompatHashes::initialize();
#endif
}

GDExtensionManager::~GDExtensionManager() {
#ifndef DISABLE_DEPRECATED
GDExtensionCompatHashes::finalize();
#endif
}
1 change: 1 addition & 0 deletions core/extension/gdextension_manager.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ class GDExtensionManager : public Object {
void reload_extensions();

GDExtensionManager();
~GDExtensionManager();
};

VARIANT_ENUM_CAST(GDExtensionManager::LoadStatus)
Expand Down
1 change: 1 addition & 0 deletions core/register_core_types.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down
3 changes: 3 additions & 0 deletions editor/editor_node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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)));

Expand Down Expand Up @@ -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;
Expand Down
25 changes: 16 additions & 9 deletions editor/editor_themes.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,15 @@ 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);
}

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

Expand Down Expand Up @@ -215,6 +215,11 @@ void EditorColorMap::create() {
add_conversion_exception("Breakpoint");
}

void EditorColorMap::finish() {
color_conversion_map.clear();
color_conversion_exceptions.clear();
}

Vector<StringName> EditorTheme::editor_theme_types;

// TODO: Refactor these and corresponding Theme methods to use the bool get_xxx(r_value) pattern internally.
Expand Down Expand Up @@ -301,13 +306,15 @@ Ref<StyleBox> 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.
Expand Down
9 changes: 6 additions & 3 deletions editor/editor_themes.h
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,14 @@ class EditorColorMap {
static HashSet<StringName> 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<Color, Color> &get_color_conversion_map() { return color_conversion_map; };
static HashSet<StringName> &get_color_conversion_exceptions() { return color_conversion_exceptions; };

static void create();
static void finish();
};

class EditorTheme : public Theme {
Expand All @@ -66,7 +68,8 @@ class EditorTheme : public Theme {
virtual Ref<Texture2D> get_icon(const StringName &p_name, const StringName &p_theme_type) const override;
virtual Ref<StyleBox> get_stylebox(const StringName &p_name, const StringName &p_theme_type) const override;

EditorTheme();
static void initialize();
static void finalize();
};

Ref<Theme> create_editor_theme(Ref<Theme> p_theme = nullptr);
Expand Down
4 changes: 4 additions & 0 deletions editor/project_manager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -2845,6 +2845,7 @@ ProjectManager::ProjectManager() {
}

EditorColorMap::create();
EditorTheme::initialize();
Ref<Theme> theme = create_custom_theme();
DisplayServer::set_early_window_clear_color_override(true, theme->get_color(SNAME("background"), EditorStringName(Editor)));

Expand Down Expand Up @@ -3297,6 +3298,9 @@ ProjectManager::~ProjectManager() {
if (EditorSettings::get_singleton()) {
EditorSettings::destroy();
}

EditorColorMap::finish();
EditorTheme::finalize();
}

void ProjectTag::_notification(int p_what) {
Expand Down