Skip to content

Handle GC cycles properly in intl iterators #17355

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

Closed
wants to merge 1 commit into from

Conversation

nielsdos
Copy link
Member

@nielsdos nielsdos commented Jan 4, 2025

This is a follow-up on GH-17343 to implement GC cycle management. Previously the objects lived too long due to the strong cycle. This patch adds get_gc handlers to break the cycle.

This is a follow-up on phpGH-17343 to implement GC cycle management.
Previously the objects lived too long due to the strong cycle.
This patch adds get_gc handlers to break the cycle.
@nielsdos nielsdos marked this pull request as ready for review January 4, 2025 15:49
@nielsdos nielsdos requested a review from devnexen as a code owner January 4, 2025 15:49
@devnexen
Copy link
Member

devnexen commented Jan 4, 2025

kind on of the fence for this particular change as landing in 8.3

@nielsdos
Copy link
Member Author

nielsdos commented Jan 4, 2025

So the main motivation is getting rid of the efree(iter); calls, which is technically not 100% correct I realized because the engine should be the ones freeing the objects.
If this doesn't land in 8.3, then I will revert GH-17343 from 8.3 too to preserve the existing destruction semantics.

@devnexen
Copy link
Member

devnexen commented Jan 4, 2025

Yes I got this, even tough stable branch is normally for real bug fixes but in the same time ...

@nielsdos nielsdos closed this in ee2faaa Jan 4, 2025
charmitro pushed a commit to wasix-org/php that referenced this pull request Mar 13, 2025
This is a follow-up on phpGH-17343 to implement GC cycle management.
Previously the objects lived too long due to the strong cycle.
This patch adds get_gc handlers to break the cycle.

Closes phpGH-17355.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants