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

Prevent read-after-free in the queued CallableCustomStaticMethodPointer, fixes slot >= slot_max errors in release templates #85280

Merged
merged 1 commit into from
Nov 23, 2023

Conversation

bruvzg
Copy link
Member

@bruvzg bruvzg commented Nov 23, 2023

Fixes #85263 and fixes #70910

@bruvzg bruvzg added this to the 4.x milestone Nov 23, 2023
@bruvzg bruvzg requested a review from a team as a code owner November 23, 2023 21:23
@akien-mga akien-mga modified the milestones: 4.x, 4.2 Nov 23, 2023
@akien-mga akien-mga added the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Nov 23, 2023
Copy link
Member

@akien-mga akien-mga left a comment

Choose a reason for hiding this comment

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

Looks good to me. I tested and it fixes the issue.

I assume the reason this was excluded in debug builds is for performance, though it would be good to benchmark whether there's a significant performance difference.

But from experience we tend to exclude too much stuff for non-DEBUG_ENABLED without profiling if it's worth it, and these discrepancies often lead to fairly bad bugs like this one.

So let's fix the bug and see if there's any negative impact on performance (which would still be better than games crashing).

@akien-mga akien-mga merged commit 4247244 into godotengine:master Nov 23, 2023
15 checks passed
@akien-mga
Copy link
Member

Thanks! This should solve a lot of pain points publishing release builds.

@bruvzg bruvzg deleted the custom_call_read_after_free branch November 24, 2023 08:23
@akien-mga akien-mga changed the title Prevent read-after-free in the queued CallableCustomStaticMethodPointer. Prevent read-after-free in the queued CallableCustomStaticMethodPointer, fixes slot >= slot_max errors in release templates Nov 24, 2023
@YuriSizov YuriSizov removed the cherrypick:4.1 Considered for cherry-picking into a future 4.1.x release label Jan 23, 2024
@YuriSizov
Copy link
Contributor

Cherry-picked for 4.1.4. (I also included some changes to error macros from #83003 which were causing conflicts when picking.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants