Skip to content

Auto-release static GDTypes at exit. #114790

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.

Sign up for GitHub

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

Merged
akien-mga merged 1 commit into from
Jan 10, 2026
+17 −1
Merged

Auto-release static GDTypes at exit. #114790

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 10 additions & 0 deletions core/object/class_db.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2420,6 +2420,7 @@ void ClassDB::cleanup_defaults() {
default_values_cached.clear();
}

LocalVector<GDType **> ClassDB::gdtype_autorelease_pool;
void ClassDB::cleanup() {
//OBJTYPE_LOCK; hah not here

Expand All @@ -2440,6 +2441,15 @@ void ClassDB::cleanup() {
resource_base_extensions.clear();
compat_classes.clear();
native_structs.clear();

for (GDType **type : gdtype_autorelease_pool) {
if (!type) {
WARN_PRINT("GDType in autorelease pool was cleaned up before being auto-released. Ignoring.");
Copy link
Member

@AThousandShips AThousandShips Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
WARN_PRINT("GDType in autorelease pool was cleaned up before being auto-released. Ignoring.");
WARN_PRINT_ONCE("GDType in autorelease pool was cleaned up before being auto-released. Ignoring.");

I'd say, as it doesn't contain any details it'll be noisy if it floods with many of these

Copy link
Member Author

@Ivorforce Ivorforce Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The warning isn't expected to ever print anyway, it's just a precaution for misuse. With multiple prints it would at least be possible to estimate how many GDTypes are affected.

Copy link
Member

@AThousandShips AThousandShips Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest replacing it with a counter in that case to make it less noisy

Copy link
Contributor

@Repiteo Repiteo Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is meant to be an impossible scenario, wouldn't DEV_ASSERT be preferable?

Copy link
Member Author

@Ivorforce Ivorforce Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have agreed a year ago, but since working on Godot more I've grown more into the "don't crash if it's not fatal" mindset, at least for Godot. Warnings should be plenty to find the situation if it ever does arise.

Copy link
Contributor

@Repiteo Repiteo Jan 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd argue that's only relevant when something's directly accessible by the end-user. But we don't clearly define if internals should be handled to a different standard, so I digress

Copy link
Contributor

@dsnopek dsnopek Jan 9, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It could also be DEV_CHECK_ONCE() if the goal is to keep this out of normal builds? I'm not sure it matters too much in this particular case, though

}
memdelete(*type);
*type = nullptr;
}
gdtype_autorelease_pool.clear();
}

// Array to use in optional parameters on methods and the DEFVAL_ARRAY macro.
Expand Down
4 changes: 4 additions & 0 deletions core/object/class_db.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,10 @@ class ClassDB {
static Array default_array_arg;
static bool is_default_array_arg(const Array &p_array);

// Types added here will be automatically cleaned up on engine shutdown.
// Only add types that aren't cleaned up in another way.
static LocalVector<GDType **> gdtype_autorelease_pool;

private:
// Non-locking variants of get_parent_class and is_parent_class.
static StringName _get_parent_class(const StringName &p_class);
Expand Down
4 changes: 3 additions & 1 deletion core/object/object.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2383,8 +2383,10 @@ void Object::assign_type_static(GDType **type_ptr, const char *p_name, const GDT
// Assigned while we were waiting.
return;
}
type = memnew(GDType(super_type, StringName(p_name, true)));
type = memnew(GDType(super_type, StringName(p_name)));
*type_ptr = type;

ClassDB::gdtype_autorelease_pool.push_back(type_ptr);
}

Object::~Object() {
Expand Down