Skip to content

Conversation

iluuu1994
Copy link
Member

Fixes GH-19844

@nielsdos
Copy link
Member

This looks hacky and will likely cause leaks for resources that were not yet destroyed.

@iluuu1994
Copy link
Member Author

iluuu1994 commented Sep 15, 2025

I'm not sure there are any good solutions for bailout during shutdown. We might minimize leaking by wrapping each resource destructor call in zend_try. I'm open to suggestions.

Also note that it at least does not introduce any new leaks. The current code would already leak if the second attempt to call the destructor bails again.

@arnaud-lb
Copy link
Member

arnaud-lb commented Sep 16, 2025

I agree this is better than before, and this is similar to how we handle bailouts in shutdown_destructors(). However there is more risk of leaking scarce resources (file descriptors) in this case than with destructors, so it may be worth it to try to release them anyway. zend_close_rsrc_list() could wrap the entire loop in zend_try and restart to the next resource after a bailout.

@iluuu1994 iluuu1994 changed the base branch from PHP-8.4 to PHP-8.3 September 17, 2025 14:02
@iluuu1994
Copy link
Member Author

Okay, I adjusted the code to keep closing resources even after bailout, so that the resources don't leak.

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.

3 participants