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 "Opcache breaks autoloading after E_COMPILE_ERROR" #8297

Merged
merged 1 commit into from
Apr 22, 2022

Conversation

arnaud-lb
Copy link
Member

@arnaud-lb arnaud-lb commented Apr 3, 2022

Fixes GH-8063 (may also fix GH-8164, GH-8358)

Recorded errors may be attached to the wrong cached script when a fatal error occurs during recording. This happens because the fatal error will cause a bailout, which may prevent the recorded errors from being cleared. If a script is compiled after bailout (or a class is linked or bound), the recorded errors will be attached to it and cached.

More specifically, zend_do_link_class may enable error recording, but will not clear recorded errors if a bailout happens. This can also happen in zend_try_early_bind. preload_link and opcache_compile_file prevent this by using zend_try.

In this change I also use zend_try in zend_do_link_class and zend_try_early_bind to ensure that recorded errors are cleared before bailout.

I was not sure about the overhead of zend_try, so I ran some benchmarks:

With opcache: https://gist.github.com/arnaud-lb/fa42613d7c18061e0eaf64c3593d1e84

Without opcache: https://gist.github.com/arnaud-lb/c10841d190113e25bdacfdf44b0bd513

patch1 is this PR, patch0 is a different implementation that clears recorded errors in php_call_shutdown_functions instead of using zend_try.

link_class and bind_class are micro benchmarks for zend_do_link_class and zend_try_early_bind (here and here, respectively). I couldn't obtain stable results for these. The results are more dependent on the order of the PHP versions. than anything else.

The Symfony demo app benchmark shows no significant difference.

Recorded errors may be attached to the wrong cached script when a fatal error
occurs during recording. This happens because the fatal error will cause a
bailout, which may prevent the recorded errors from being freed. If an other
script is compiled after bailout, or if a class is linked after bailout, the
recorded errors will be attached to it.

This change fixes this by freeing recorded errors before executing shutdown
functions.

Fixes phpGH-8063
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