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

PHP OPCache should be reset after WordPress upgrader_process_complete #740

Closed
raamdev opened this issue Apr 18, 2016 · 6 comments
Closed

PHP OPCache should be reset after WordPress upgrader_process_complete #740

raamdev opened this issue Apr 18, 2016 · 6 comments
Assignees
Milestone

Comments

@raamdev
Copy link
Contributor

@raamdev raamdev commented Apr 18, 2016

As of v160417, Comet Cache Pro automatically clears the PHP OPCache whenever it detects a new version of itself, however Comet Cache Lite does not (this was an oversight, not intentional).

As a result, Comet Cache Lite is more prone to fatal errors when upgrading from an older version of Comet Cache that contains major changes, such as a change from PHP Closures (v160227) to PHP Traits (v160417).

In addition to making sure that Comet Cache Lite clears the PHP OPCache after an update, we should reset the PHP OPCache after any WordPress Core or Plugin/Theme update, as it's possible for any WordPress update to introduce code that might trigger a fatal error during that 2-second window.

We should attach to the WordPress shutdown hook and call opcache_reset() whenever a update has occurred. Also, it should be possible to override this new behavior just in case an advanced site owner wants control over the way this works.

What is OPCache? What is the problem?

PHP OPCache is a PHP opcode cache that many web servers run to improve PHP performance by temporarily caching the PHP code itself (this is quite different from what Comet Cache does, which is cache content generated by WordPress).

Let's say there is Comet Cache v160227 code in AbsBaseAp.php that gets cached by OPCache. You then upgrade to Comet Cache v160417, which changed the code in AbsBaseAp.php in a way that is not compatible with the old version of Comet Cache. If WordPress tries to load AbsBaseAp.php and PHP finds that AbsBaseAp.php has already been cached, it will try to load the cached version of that code (i.e., the old v160227 code), and since the v160227 code is not compatible with Comet Cache v160417, a PHP Fatal error is produced.

The way that PHP OPCache prevents this sort of problem is by revalidating the PHP files regularly to see if the code they contain has changed. It checks the timestamp on the PHP file (e.g., AbsBaseAp.php) to see if it has changed since it was last cached. If it has changed, then it clears the existing cache for that file. However the default revalidation frequency (opcache.revalidate_freq) is 2 seconds, which means there's a 2-second window of opportunity for conflicting code to be present in memory.

The way you prevent a fatal error from occurring during this 2-second window is to clear the OPCache before you introduce updated code that might conflict with a previous version.

@raamdev
Copy link
Contributor Author

@raamdev raamdev commented Apr 28, 2016

@jaswsinc This issue is specifically addressing Comet Cache Lite, which as of v160417 doesn't include the routines to clear the OPCache after upgrading, however #755 seems to indicate we may have an OPCache-related issue after upgrading Comet Cache Pro via the Pro Updater.

Maybe we need to clear the OPCache earlier than we do now?

@jaswrks
Copy link

@jaswrks jaswrks commented Apr 28, 2016

Maybe we need to clear the OPCache earlier than we do now?

Agree. To my knowledge, there's no hook attached to the pro upgrade routine itself that would clear the OPcache after the upgrade occurs. It's not until after the upgrade is complete, and a new pageview occurs that we detect a new version and run the automatic update routines. By that time, the additional pageview with a stale opcode cache may have already created an issue.

Whenever we add the shutdown hook that should cover this scenario also though.

@raamdev
Copy link
Contributor Author

@raamdev raamdev commented Apr 28, 2016

Noting some earlier discussion from Slack:

@raamdev writes...

we can attach to upgrader_process_complete to see when an update has occurred. We already attach to that hook to run autoClearOnUpgraderProcessComplete() to clear the cache in Comet Cache.

https://github.com/WordPress/WordPress/blob/4.5/wp-admin/includes/class-wp-upgrader.php#L1101-L1122

I'm thinking we can add a line right below here that attaches our existing wipeOpcache() to the WordPress shutdown hook: https://github.com/websharks/comet-cache-pro/blob/160417/src/includes/traits/Plugin/WcpUpdaterUtils.php#L53

(And similarly after other $upgrading_active_* = true; conditions in that autoClearOnUpgraderProcessComplete() routine.)

@jaswsinc writes...

Yes, that looks like it would work well. Cool!

Yeah. Looks like here:
https://github.com/websharks/comet-cache-pro/blob/160417/src/includes/traits/Plugin/WcpUpdaterUtils.php#L93
and here too: https://github.com/websharks/comet-cache-pro/blob/160417/src/includes/traits/Plugin/WcpUpdaterUtils.php#L99

@raamdev raamdev self-assigned this Apr 28, 2016
raamdev added a commit to wpsharks/comet-cache-pro that referenced this issue Apr 28, 2016
raamdev added a commit to wpsharks/comet-cache-pro that referenced this issue Apr 28, 2016
raamdev added a commit to wpsharks/comet-cache-pro that referenced this issue Apr 28, 2016
raamdev added a commit to wpsharks/comet-cache-pro that referenced this issue Apr 29, 2016
@raamdev
Copy link
Contributor Author

@raamdev raamdev commented May 10, 2016

Next Release Changelog:

  • Bug Fix: When the PHP OPCache extension is active, the OPCache is now cleared when a WordPress plugin is upgraded, activated, or deactivated. This works around an issue that could produce a fatal error when the PHP OPCache contains cached PHP code that conflicts with new PHP code introduced by an update. See Issue #740.
@raamdev raamdev closed this May 10, 2016
@raamdev
Copy link
Contributor Author

@raamdev raamdev commented May 21, 2016

Comet Cache v160521 has been released and includes changes from this GitHub Issue. See the v160521 announcement for further details.


This issue will now be locked to further updates. If you have something to add related to this GitHub Issue, please open a new GitHub Issue and reference this one (#740).

@wpsharks wpsharks locked and limited conversation to collaborators May 21, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
2 participants