Invalidate files in opcode cache after plug-in, theme or core update

[nigro.simone]

wordpress it seem not reset Zend OpCache after plug-in, theme or core are updated.

For solve this issue i have set in the php.ini

opcache.validate_timestamps=1

all work correctly but validate_timestamps when it's enabled, PHP will check the file timestamp each request with a performance degradation. When it's disabled, PHP files are NEVER checked for updated code. When wordpress updating code, new code files can get mixed with old ones, the results are unknown. It's unsafe as hell.

Why wordpress not perform an opcache_reset() after each update if opcache is active and opcache.validate_timestamps is false?

[dd32]

When running in an environment where PHP files are modified, using an opcode cache without some kind of filesystem monitoring is always going to be a bad idea - the opcode cache simply is not designed to be used within that environment.

I'm not against adding this, especially as it's more likely to be enabled in PHP 7, but this won't help at all for scenario's where a manual update is run, or WP-CLI (or other similar tools) are used to manage the updates.

It'd have to be run post plugin/theme/core update, on the plugin/theme editors, probably needs some way of being triggered by plugins too, etc.

[JasWSInc]

+1 for trying to automatically flush the OPcache (in core) after any theme/plugin update occurs. Perhaps on the shutdown action hook once the update is complete.

[szepe.viktor]

Please be aware that opcache.restrict_api might be set preventing execution.

[swissspidy]

Adding this to the 4.8 milestone because we've seen many reports of broken sites after updates, all related to the OpCache not being cleared.

​https://wordpress.org/support/topic/wordpress-4-7-fatal-error-cannot-redeclare-get_paged_template/ is just one example.

[JanR]

Currently I'm looking into our opcache configuration settings in PHP 5.6 and 7, as we see more and more issues with WordPress updates lately too.

To ease updates you can use the upgrader_pre_install() hook, and perform a flush of the cache. I've created a little Must Use Plugin [1] for this (it cannot be done from within a plugin when using upgrader_pre_install()).

And don't forget OPcache's file path caches: ​http://php.net/manual/en/function.opcache-get-status.php.

As long as tools like WP-CLI use native WordPress core functionality, there should be no problem adding such a flush function to the core.

I'll get back when and if I found some valuable opcache settings administrators may need to set.

[1] ​https://www.saotn.org/wordpress-plugin-flush-php-opcache/

[szepe.viktor]

Replying to JanR:

As long as tools like WP-CLI use native WordPress core functionality, there should be no problem adding such a flush function to the core.

OPcache uses the process' shared memory thus Apache has no common OPcache with php-cli.

Drush uses a special URL to purge opcode cache.

[JasWSInc]

Regarding opcache.restrict_api. This INI setting, if not empty, contains a filesystem path that effectively becomes a whitelist, and therefore blacklists anything outside of the filesystem path given.

An example value for opcache.restrict_api would be something like:
opcache.restrict_api = /var/www/html/wp-content/plugins/xyz/

Which states that any PHP file that has a filesystem path that begins with /var/www/html/wp-content/plugins/xyz/ is whitelisted. Any that does not, is blacklisted.

So to check this in PHP (against the current __FILE__), use something like:

<?php
if(!ini_get('opcache.restrict_api') || stripos(__FILE__, ini_get('opcache.restrict_api')) === 0) {
    opcache_reset();
}

[asalce]

I attached a new function wp_opcache_reset which should be called on "upgrader_process_complete" action. This should clear the OP Cache when ever the upgrade/update process is completed, I wasn't sure if this action is called for all updates (theme, plugin, wp core). Let me know if the attached patch fixes your issue.

[JanR]

Replying to asalce:

I attached a new function wp_opcache_reset which should be called on "upgrader_process_complete" action. This should clear the OP Cache when ever the upgrade/update process is completed, I wasn't sure if this action is called for all updates (theme, plugin, wp core). Let me know if the attached patch fixes your issue.

A patch like this is something WordPress definitely needs. However, sometimes files are locked on the file system by the opcode cache (whether that's OPcache, WinCache or APCu doesn't matter), and the flush needs to be executed prior to updating.

Also, don't forget to remove, or invalidate, cached script paths:

<?php
foreach( $opcache_status['scripts'] as $key => $data ) {
        $dirs[dirname( $key )][basename( $key )] = $data;
        opcache_invalidate( $data['full_path'] , $force = true );
}

Reference ​http://php.net/manual/en/function.opcache-get-status.php.

[asalce]

Replying to JanR

So ultimately the reset / cache_invalidation needs to happen before the old files are deleted or overwritten? I can check it out tonight and see if I can pin-point this moment within the Upgrader Class.

[asalce]

The closest filter I found was 'upgrader_pre_download' which fires a few steps before the files are extracted from the zip file. I attached '36455.3.diff' with the proper use of the filter (ignore patch 2).

I also added some checks in the filter so:

1. It only clears files within Wordpress' base dir
2. Trigger only when validate_timestamps is not active

@JanR if we are invalidating each file, do we still need to call opcache_reset?

@dd32 brings up a good point about the external plugin updates. I am not sure there is much that can be done here, these tools should be made aware to call wp_opcache_reset manually. There is also the case when updates are ran via the filesystem (FTP for instance), which will completely by-pass all of Wordpress/PHP. My best guess is that certain hosts whom have validate_timestamps deactivated should take extra precaution when deploying to production, probably part of their build procedures.

[asalce]

Fixed a typo. No more coding at 2AM for me! :)

[Garavani]

This helped me out with my blank page issue after updating to wp 4.7.2. Thanks so much!!!
Anyhow I had to create a php.ini file first in the wp-admin directory.

[pavelevap]

What about plugin deactivation? It seems to me that scripts from deactivated plugins has the same problem (still in Opcache memory).

[jbpaul17]

Punting to 4.8.1 per today's bug scrub.

[kirasong]

Just took a look at 36455.4.diff​ .

Thinking we should still clear when opcache.validate_timestamps = 1, because this means that opcache checks the timestamps, but the invalidation only happens every opcache.revalidate_freq seconds, so we may still have invalid items in the cache: ​http://php.net/manual/en/opcache.configuration.php#ini.opcache.validate-timestamps

I'd be interested to know about what happens there is an invalidation attempt and opcache.restrict_api restricts this. Namely, is there a way to catch failure rather than checking beforehand using ini_get()?

[szepe.viktor]

<?php
$return = opcache_invalidate('/bla/bla.php');
var_dump($return);

It says: bool(false)
And in the error log: PHP Warning: Zend OPcache API is restricted by "restrict_api" configuration directive in /bla/bla.php

[szepe.viktor]

The PHP docs says: Returns TRUE if the opcode cache for script was invalidated or if there was nothing to invalidate, or FALSE if the opcode cache is disabled.

[jbpaul17]

Punting to 4.9 per today's bug scrub.

[jbpaul17]

Punting to Future Release per today's 4.9 bug scrub.

[pputzer]

Any news on this? I just got hit again by the auto-upgrade to 4.9.2, despite a MU plugin with 36455.4.diff being active.

[GregLone]

Hi.
This is something we're also struggling with, after our plugins being updated. It always ends in a fatal error because files have been removed in new versions.
I hope to this one coming soon in core.
Thank you folks.

[benoitchantre]

Replying to mikeschroder:

I'd be interested to know about what happens there is an invalidation attempt and opcache.restrict_api restricts this. Namely, is there a way to catch failure rather than checking beforehand using ini_get()?

I wasn't able to configure opcache with restrict_api.

My tests were done using recommended opcache settings for production from ​Scaling PHP Book

opcache.revalidate_freq=0
opcache.validate_timestamps=0 (comment this out in your dev environment)
opcache.max_accelerated_files=7963
opcache.memory_consumption=192
opcache.interned_strings_buffer=16
opcache.fast_shutdown=1

Let me know if something need to be adjusted.

[jadonn]

Some more research may be worthwhile before integrating this into core WordPress. Under certain PHP configurations, calling opcache_reset will reset the opcache for all users on a given server, or, at the very least, all websites that a given user owns. For example, if a server using PHP-FPM uses a common PHP-FPM master process, despite having separate pools of workers for each worker, calling opcache_reset will reset the opcache for all users on the server using the common PHP-FPM master process. I believe this same behavior occurs when using mod_php (running PHP as a Dynamic Shared Object in Apache) even when using mod_ruid2 or other security modules in Apache, but it has been a little while since I last worked with a server running mod_php/DSO for PHP handling.

Clearing the opcache for all websites on a server or all websites owned by a single user would likely have a pretty significant impact on performance in terms of traffic that could be served and on website resource usage, outside of the security concerns, while the cache is being rebuilt.

There are some real security concerns around opcache and from incorrectly configuring opcache due to how opcache memory is shared, but those may be better addressed elsewhere.

[pputzer]

Replying to jadonn:

Clearing the opcache for all websites on a server or all websites owned by a single user would likely have a pretty significant impact on performance in terms of traffic that could be served and on website resource usage, outside of the security concerns, while the cache is being rebuilt.

Well, the alternative is a mysteriously broken site (including white screens, if the wrong things change in a plugin), so I don't think any possible performance impact is salient (especially since it would only be temporary).

[GregLone]

Hi there,

You'll get a fatal error with php < 5.5 because of empty():

if ( ! empty( ini_get( 'opcache.restrict_api' ) ) && strpos( __FILE__, ini_get( 'opcache.restrict_api' ) ) !== 0 ) {
        return;
}

Should be:

$restrict_api = ini_get( 'opcache.restrict_api' );

if ( $restrict_api && strpos( __FILE__, $restrict_api ) !== 0 ) {
        return;
}

[benoitchantre]

Replying to GregLone:

Hi there,

You'll get a fatal error with php < 5.5 because of empty():

if ( ! empty( ini_get( 'opcache.restrict_api' ) ) && strpos( __FILE__, ini_get( 'opcache.restrict_api' ) ) !== 0 ) {
        return;
}

Should be:

$restrict_api = ini_get( 'opcache.restrict_api' );

if ( $restrict_api && strpos( __FILE__, $restrict_api ) !== 0 ) {
        return;
}

Before this test, there's a guard clause that check for the existence of opcache_reset() (since PHP 5.5).
There should be no error with PHP < 5.5 because opcache_reset() doesn't exist in these versions and the function will return early.

Did you experiment a fatal error?

[GregLone]

Uh, you're right.
Yes I got one, but because I wrote my tests differently, and shouldn't.
My apology :)

[doc987]

Replying to pputzer:

I think there's another alternative. Updating files does not require a full opcache reset. All that's really needed is to invalidate the opcache entries for the files that change. Doing that would (a) leave the opcache intact for all other files, thereby avoiding the performance issue, and (b) prevent a broken site. The new files could even be immediately loaded to the opcache. The downside of course is that the invalidating is more complicated. Resetting the whole opcache is as simple as calling ​opcache_reset().

A single file can be invalidated with ​opcache_invalidate(). A single directory tree might have the corresponding opcache entries (if any) updated with something like the function below.

<?php
function compile_dir($path,$force=false){
   $iter=new \RecursiveIteratorIterator(new \RecursiveDirectoryIterator($path));
   foreach($iter as $file){
      $ext=$iter->getExtension();
      if(in_array($ext,['php','svg',/* other file extensions */])){
         $name=$file->getPathname();
         $cached=opcache_is_script_cached($name);
         if($force && $cached) opcache_invalidate($name);
         if($force || !$cached) opcache_compile_file($name);
      }
   }
}

Invalidating files upon update has the additional performance advantage of allowing ​opcache.validate_timestamps to be set to 0.

If there was a problem, I suppose a logged in user could be given a button to force an opcache reset. Too bad the opache is not an LRU cache (​https://github.com/zendtech/ZendOptimizerPlus/issues/187).

[kraftbj]

I'm definitely seeing an increase of reports of fatals that I can only attribute to opcache having out-of-date files present. A check for a class, followed by a call to a method within that class that was added sometime after the file was originally included in the plugin, and verifying in the file system that the correct version of the class with that method is present.

[Krstarica]

Any update on this? This is still happening with every update.

[kirasong]

I agree that we should really fix this.

Self-assigning to look into for 5.4, but any further testing of the existing patches/other approaches are appreciated, and I'm happy to assign elsewhere if someone has time.

[davidbaumwald]

@mikeschroder Is this one still on your list for 5.4?

[kirasong]

@davidbaumwald I brought this up with the hosting team.

Here's my opinion in the meantime:
I agree with a few folks in here that ideally the flush would be targeted to the exact files that changed. This would help avoid causing unnecessary churn on servers where there are a bunch of auto-updates happening at the same time.

I don't think I'll have time to write something up like that before beta, unfortunately. *Ideally* this is something that would get committed before beta so it has a bit of time to soak.

I'm mixed on whether committing something like 36455.5.diff​ in the meantime would be more helpful or harmful, and will comment back when I hear from some folks in the hosting team!

[davidbaumwald]

@mikeschroder Any feedback to add here yet? Beta 1 is tomorrow, and we need a decision on this soon.

[kirasong]

So far, have heard from one more host that has mentioned they also think it'd be a good idea to handle this per file.

For this reason, going to go ahead and punt it to next release.

I'm assigned to this to keep an eye on it (and hope to have time before next release to dig in further), but if anyone else would like to submit a patch or be assigned, please feel free to do so! I think this is one of those tickets where the most possible approaches would be great.

Edit: Deleted duplicate comment.

[GregLone]

Hi,

We use something similar in our plugins and it works like a charm.
But recently we had to use ini_get( 'opcache.enable' ) in the conditions, as a client reported an issue. I just want to outline this.

[Krstarica]

Two plugins doing the similar thing could be helpful:
​https://wordpress.org/plugins/clear-opcache/
​https://wordpress.org/plugins/opcache/

[jnylen0]

More info on the recommendation to avoid opcache_reset: ​https://serverfault.com/questions/959520/php-fpm-generated-a-lot-of-load-when-resetting-opcache-rendering-server-unrespo

[jnylen0]

attachment:36455.6.2.diff​ is a preliminary patch that calls opcache_invalidate() on PHP files as they are written.

So far we've done some light testing on this using the ClassicPress migration plugin. If you're having trouble reproducing this issue otherwise, this is a good way to test because many files are changed when moving from WP to CP. More info here: ​https://github.com/ClassicPress/ClassicPress/pull/567

This still needs more testing, for example with opcache.restrict_api set (though if this is just a normal PHP warning then the @ operator seems like an appropriate choice), and with plugin/theme updates.

[kirasong]

Hey @jnylen0 -- thanks so much for the patch, and apologies for the long time to reply!

I'll see if I can get some testing in the WP hosting community going, so we can land this early if it seems to be working well for folks.

[nextendweb]

@jnylen0 Is there any reason to apply a filterwp_opcache_invalidate_file to every file? I do not see why someone want to exclude files which are changed from the invalidation.

[davidbaumwald]

@mikeschroder Were you able to have this tested by the hosting community? Is this still in the cards for early in 5.5, given that the cycle is ​officially kicked off now?

[kirasong]

Replying to davidbaumwald:

@mikeschroder Were you able to have this tested by the hosting community? Is this still in the cards for early in 5.5, given that the cycle is ​officially kicked off now?

I haven't got the feedback I'd like (except "yes we should do this"), but I think there's still time for the cycle in terms of stability (the reason for the early tag).

Have this on my 5.5 priorities, and would like to get something landed that can iterate a little during alpha. Any attention as far as feedback from folks would be appreciated, though.

[nextendweb]

36455.7.diff

[nextendweb]

I proprose 36455.7.diff:

• the filter in the previous patch is not necessary in my opinion. I do not see any kind of usage of that as if a plugin/theme/wordpress gets updated, just other plugins/themes/(previous wp version) could exclude any file.
• Optimized function_exists( 'opcache_invalidate' ) call to run only once per install
• Added .phtml file extension as it is very popular. Consideration: These are valid file extensions for PHP, but I think they are very rare: .php3 .php4 .php5

BTW: I'm thinking about that maybe it would be better to call opcache_invalidate() on the deleted PHP files also before the deletion. Maybe that would be a more safe way of clearing the opcache.

Also I think none of these steps solve the issue when the user just uploads(ssh,ftp,sftp) the update/plugin/theme directly and in that case the site still can crash because the invalid cache state.

[jnylen0]

Replying to nextendweb:

I proprose 36455.7.diff:

in_array should set the optional third parameter to true to use a strict comparison.

the filter in the previous patch is not necessary in my opinion.

I recommend keeping the filter. It is mainly intended for hosting providers that want to override this logic for whatever reason, without fully disabling the opcache APIs. WordPress.com is an example of a provider that might want to do this. Also, just in general, more flexibility is better than less.

BTW: I'm thinking about that maybe it would be better to call opcache_invalidate() on the deleted PHP files also before the deletion. Maybe that would be a more safe way of clearing the opcache.

Seems worth investigating.

It might also be worth investigating whether a file has actually changed before invalidating its cache - I don't know how much of a performance impact that would have, but I think in the current patches all files in the plugin/theme/core update will be invalidated.

[nextendweb]

36455.8.diff

[nextendweb]

In 36455.8.diff:

Changed the suggested in_array strict param to true

If a filter is really needed, I'm not sure still the reason why a host would want to allow one file to be cleared an not the others. If a plugin has a lot of PHP files, it would take a lot of time to run it on each. So I propose to add a single filter where you can disallow the clearing of the opcache.

<?php
$opcache_invalidate_allowed = function_exists( 'opcache_invalidate' ) && apply_filters('wp_opcache_invalidate_allowed', true);

It might also be worth investigating whether a file has actually changed before invalidating its cache - I don't know how much of a performance impact that would have, but I think in the current patches all files in the plugin/theme/core update will be invalidated.

​https://www.php.net/manual/en/function.opcache-invalidate.php

If the second optional parameter left on the default false, then PHP will only clear its cache if it changed. So I do not think that we should make extra effort to accomplish the same thing.

[davidbaumwald]

@mikeschroder Does the latest patch look like a good approach to you? What else needs to be done here for this to land in 5.5?

[williampatton]

Patch refreshed with minor PHPCS fixes

[ayeshrajans]

I work on a Composer Opcache preload plugin, and personally spend a lot of time with opcache, and it is quite uncommon to see timestamp-validation turned off. I believe that if someone the validation off, they must be aware of the consequences of it.

Regarding the patch, I would like to suggest a few changes and considerations:

1. Statically cache the result whether opcache should be reset or not. This prevents applying wp_opcache_invalidate_allowed filter and other ini_get/function_exists calls over an over.

2. In extension check, a simple regular expression can be nearly 15 times faster compared to the in_array(pathinfo()) combo. See a simple benchmark here: ​https://3v4l.org/Qv39v

3. If opcache.validate_timestamps is set 1, and opcache.revalidate_freq set to 0, we do not need to clear opcache for that file because PHP will need to revalidate opcache by itself on every request. By default, opcache.revalidate_freq is set to 2 seconds.

4. It is unlikely that this is the only case that we need to clear opcache for a file, so ideally we should create a reusable function to do this elsewhere in the code. See #50354

5. There are currently 2 opcache_invalidate calls in core.

I worked on patch that takes a slightly different approach to address all 5 points above (attached in next comment).

[ayeshrajans]

Same patch as 36455.9.patch, but +1 patch number because 36455.9.diff already exists. Sorry.

[nextendweb]

@ayeshrajans we just released an update to our plugin, had around 100k updates in 2 weeks. Our support staff had around 10 tickets related to the opcache (disabled validate_timestamps or huge revalidate_freq). Also there might be others who experienced this problem, but they were able to solve it on their own. So it is not so common, but I'm sure it happened a lot of people in the past and they are just getting some random PHP error which do not give any hint how to solve it.

Personally I would remove this part from your code to make sure that it is cleared all the time. I think when opcache_invalidate function is available we should call it, as it does what its need to be done now or later.

( ! ini_get( 'opcache.validate_timestamps' ) || ini_get( 'opcache.revalidate_freq' ) === "0" )

Otherwise, I like your changes.

[ayeshrajans]

Thanks for the quick response @nextendweb. I understand your point clearing cache eagerly should get priority over having broken code.

There was a mistake in 36455.10.patch, that it did not properly check the invalidation criterion. I will follow-up with 2 patches:

• 36455.11.patch

This one is same as 36455.10.patch, but now correctly determines if the cache should be cleared. opcache.validate_timestamps === 0, or any non-zero opcache.revalidate_freq value now triggers the cache to be cleared.

• 36455.11-no-timestampcheck.patch

Same patch, but without the timestamp/freq precautions. This takes a more wildcard approach.

I'm personally a bit hesitant to clear opcache unless we absolutely have to. Clearing opcache itself should be faster because it only clears the memory, but the next time the file needs to be cached, it requires the disk hit which might be catastrophic specially on shared servers if an automated update is triggered in a short time-span.

[szepe.viktor]

it is quite uncommon to see timestamp-validation turned off.

Yes!! These uncommon setups are almost 100% agency-type projects in highly controlled environments.

But an agency type project has auto-update turned off :) so the Deploy process takes care of file changes AND OPcache purging.

[nextendweb]

@ayeshrajans opcache_invalidate -> "the script will only be invalidated if the modification time of the script is newer than the cached opcodes." ​https://www.php.net/manual/en/function.opcache-invalidate.php

Probably if we update a plugin the modification time will change for every file as WordPress deletes the whole folder and replaces it with the new version of the whole plugin.

1. If we call opcache_invalidate for every file: you are right, PHP will recompile them when they used next time.

2. If do not call opcache_invalidate when timestamp enabled: We do not tell PHP to invalidate the files, but it will invalidate them after the revalidate_freq so the result will be the same as 1., but there will be a timeframe between the install of a plugin/theme/core and the revalidate_freq while the opcode cache is in invalid state.

PHP will do the same thing if revalidate_freq enabled, so if a shared hosting crashes if we call opcache_invalidate on every case, then it would crash when the revalidate_freq notice the changes for all of those files.

[kirasong]

Replying to davidbaumwald:

@mikeschroder Does the latest patch look like a good approach to you? What else needs to be done here for this to land in 5.5?

Thanks for the ping! I love the progress on this ticket -- thanks so much everyone! The biggest change I had been thinking was to have a helper function, which looks to be in the most recent patch.

I'll review/test tomorrow.

[kirasong]

Okay! Working on a pass here.

Not quite done, but I said I'd review today, so I wanted to post an update.

Currently, I'm liking a combination of the approaches a lot -- having a filter available within a helper function, to allow hosts to do this in a custom manner, and invalidating whether or not revalidate_freq is set, to cover the case of a race condition before files are invalidated by PHP automatically.

After a chat with @aaroncampbell, I'm not sure if it's necessary to invalidate deleted files, since unless missing something, if the new/modified files are invalidated properly, they should no longer reference those deleted files.

I'm trying to think of cases where invalidating deleted files would be important. Direct requests to them that are started from an already loaded page (before the update), perhaps? Are there any common cases y'all have seen?

[nextendweb]

Replying to mikeschroder:

I'm trying to think of cases where invalidating deleted files would be important. Direct requests to them that are started from an already loaded page (before the update), perhaps? Are there any common cases y'all have seen?

I can not think any case where not invalidating deleted files would result in error. The idea behind this, to release opcache memory for the files which are deleted. Probably opcache itself do garbage collection for the long time not used files. If that is the case, there is no need to invalidate deleted files.

It is worth to mention that opcache will resets itself completely when it reaches the opcache.max_accelerated_files limit, but I'm not sure how opcache_invalidate affects this limit:
​https://serverfault.com/questions/730953/php-opcache-is-resetting-cache-automatically

Update: Just checked the source code of the Opcache in the PHP repo and it looks like if we call opcache_invalidate on the deleted files that does not free up space in the opcache.max_accelerated_files limit, but it frees up memory/storage as it deletes the opcache of the deleted file. But I do not think that it would have a huge impact.

[kirasong]

Helper function and filter. Static extension checking. Modified docs.

[szepe.viktor]

Hello! I'm unable to un-watch this ticket.
Notification emails keep coming

Re: [WordPress Trac] #36455: Call opcache_reset() after plug-in, theme or core update

Please advise.

[kirasong]

Thanks so much @nextendweb!

Did a pass on this in 36455.12.diff​, and would appreciate a look/test/opinions from whoever has the chance.

I'm noticing that this widens the files that might be invalidated in the plugin editor to include .phtml.

I didn't include the parameter for extension in this pass -- It might be worthwhile to check and see how common the alternate names are in the plugin/theme repos, to see what should be whitelisted, and how much it'd help users to be able to specify an extension.

Edit: A quick note that @stevenkussmaul helped out with this patch as well, for future props by whoever is the committer.

[nextendweb]

@mikeschroder I think there is a missing () in your code. Checks for the opcache.restrict_api should be wrapped as 0 && 0 || 1 evaluates to true even when opcache_invalidate is not available.

$can_invalidate = function_exists( 'opcache_invalidate' ) && ( !ini_get( 'opcache.restrict_api' ) || stripos( __FILE__, ini_get( 'opcache.restrict_api' ) ) === 0);

Also I still do not see any reason to run wp_opcache_invalidate_file filter on every file. Could you give any when you do not want to invalidate a single file or a list of files in the opcache?

[szepe.viktor]

Please ban me from this ticket!

[kirasong]

Replying to nextendweb:

@mikeschroder I think there is a missing () in your code. Checks for the opcache.restrict_api should be wrapped as 0 && 0 || 1 evaluates to true even when opcache_invalidate is not available.

Good catch. Thanks! I'll fix this.

Also I still do not see any reason to run wp_opcache_invalidate_file filter on every file. Could you give any when you do not want to invalidate a single file or a list of files in the opcache?

Sure. This is in here to allow hosts or plugins to override the behavior. This might be important if a plugin can handle its own purging better than core knows, or for hosts and caching plugins that have custom handling. I'm not positive this is the best way to allow for it, and it's possible the filter would be better elsewhere. Caching implementations can be pretty different, and this allows for the previous behavior when this comes up.

On another note:
I'm working on some of the docs, too, for the next patch. In the previous patch I left $force as the name of the variable because it matches with opcache_invalidate(). I'm wondering if something like $skip_timestamp_validation might be better, since invalidation is not forced (if it is unavailable, for instance)?

[kirasong]

Fixed logic for detecting whether invalidation is possible. Updated and added to docs.

[kirasong]

Added 36455.12.2.diff​.
This updates the logic per @nextendweb's note, and updates some of the docs.

A couple notes:

• I'm going to reach out to the hosting community team re: filter asks, to see if there are any recommendations what/how much is necessary, to give some more context to decisions there.
• This could really use tests, but much of the testing relies on changing PHP settings during a test. I'm not sure how to best do this in phpunit yet. If anyone has experience with it, that'd be welcome.

Lastly, a quick mention for everyone else following that ​we asked about @szepeviktor's note re: notifications, and it looks like there is unfortunately there is an ​open bug. I let them know via Slack, but wanted to be sure folks in here knew there was action.

[ayeshrajans]

Quoting from the slack channel on my own reply:

Looking at the patch (12.2)... Do we absolutely need to run the hook on every file? Ideally, I think we can run the hook once to get boolean whether we need to clear opcache or not, and cache that result. I highly doubt per-file opcache clear feature is necessary, and if that is the case, there are post-update hooks that plugins can selectively clear caches of.

Patch in 11 is doing the hook-once-then-cache approach. I think more verbosely commented function in 12.2 is much better, save for the concern of running the hook for every file.

We can also return-early if the static $can_invalidate is false before running the preg_match call.

[boogah]

Thanks for asking for feedback on this @mikeschroder!

As it stands, I have two concerns with the invalidating the whole cache being the default behavior here:

1. Performance on larger, high load sites. Getting rid of an entire warmed cache because a few files changed seems to be throwing out the baby with the bath water. The cache should be holding onto what it can for as long as is reasonable to help reduce overhead on the server.
2. In some cloud hosting configurations, invalidating the entire cache may cause every single file on a host to reset. That's definitely not optimal.

If we have the means to do per-file, I don't see why we shouldn't attempt to do that.

[nextendweb]

Replying to boogah:

Thanks for asking for feedback on this @mikeschroder!

As it stands, I have two concerns with the invalidating the whole cache being the default behavior here:

1. Performance on larger, high load sites. Getting rid of an entire warmed cache because a few files changed seems to be throwing out the baby with the bath water. The cache should be holding onto what it can for as long as is reasonable to help reduce overhead on the server.
2. In some cloud hosting configurations, invalidating the entire cache may cause every single file on a host to reset. That's definitely not optimal.

If we have the means to do per-file, I don't see why we shouldn't attempt to do that.

Only that file gets invalidated which has just written into the filesystem and the modification time differs from the one in the opcode cache. Not the whole opcode cache cleared.

Maybe we should change the summary of this and remove opcache_reset as it might confuse people.

[kirasong]

Thanks @nextendweb for updating the summary!

Replying to boogah:

Thanks for asking for feedback on this @mikeschroder!

As it stands, I have two concerns with the invalidating the whole cache being the default behavior here:

Thanks!

As @nextendweb mentioned, the approach currently being considered only invalidates per file.

What do you think about the approach in the latest patch​?

[jorbin]

Let's get this in so it can get wider testing.

With JIT coming in php8, there may need to be some follow-up work, but this still seems great and trunk seems like the right place to test it.

[ayeshrajans]

Based on the latest patch, I made a few improvements:

• Moved the regex file match to happen after the static $can_invalidate value check. This will prevent the regex from being executed if WordPress already knows to _not_ clear the cache.

• Removed phtml file extension from the list. I don't know why the original author added it, but I checked whole WordPress source to make sure we don't have any .phtml files.

• Optimized the regular expression to ensure the string is terminated. This to prevent files like .phpunit.result.cache files from being opcache-cleared.

[nextendweb]

@ayeshrajans Removed phtml file extension from the list. I don't know why the original author added it, but I checked whole WordPress source to make sure we don't have any .phtml files.

WordPress itself might not use it, but plugins and themes might and I'm sure they do (including myself). It would be better to keep every popular PHP file extensions to match for the best result, this is why I suggested the invalidate .php and .phtml files.

[kirasong]

An update:

• During testing before commit, I noticed an issue with Warning: Zend OPcache API is restricted by "restrict_api" configuration directive in WP-CLI during plugin or core upgrade, even though it is being run from a folder within opcache.restrict_api (and with opcache.enable_cli enabled). Still troubleshooting that. Any testing is appreciated.

• It's tricky to test the upgrade-core.php change. I'm still learning more about the way this works. Do I understand correctly that wp_opcache_invalidate() would not be available, at the very least the first upgrade around, unless it is inside upgrade-core.php? cc: @jnylen0

I'm guessing the second is part of the reason for not having a helper function in the initial patch, if I'm reading the code properly. Additional information or testing appreciated.

Replying to nextendweb:

WordPress itself might not use it, but plugins and themes might and I'm sure they do (including myself). It would be better to keep every popular PHP file extensions to match for the best result, this is why I suggested the invalidate .php and .phtml files.

I did a couple of searches about this in the plugin and theme repos.

​`phtml` or ​`\.phtml` matches in plugins.
​`phtml` or ​`\.phtml` matches in themes.

I'm sure this is not 100% accurate, but it doesn't seem to be very common in plugins/themes. Are there other ways or projects that you've been seeing .phtml used a lot?

After chatting with a few folks, I'm leaning towards only including .php initially, with adding .phtml if it looks like there were are a lot of cases missed. I've also ​reached out to the WordPress core PHP team for a recommendation.

[nextendweb]

Well, if .phtml won't get included, I will be able to rename the files in Smart Slider 3. .phtml files are popular in the Magento community. I thought there is wider usage in the repos.

Is it possible that CLI still has a different opcache? It might be related to your WP-CLI issue. Read the comments here: ​https://www.php.net/manual/en/function.opcache-reset.php

[kirasong]

Replying to nextendweb:

Well, if .phtml won't get included, I will be able to rename the files in Smart Slider 3. .phtml files are popular in the Magento community. I thought there is wider usage in the repos.

That's great, thank you -- and sorry for the difficulty. Let's see what the PHP team says, too.

Is it possible that CLI still has a different opcache? <snip>

Thanks!! I think I figured out what's going on with it from the command line, and am working on + testing a patch to change __DIR__ to realpath( $_SERVER['SCRIPT_FILENAME'] ) now. Here's my current understanding:

PHP compares with opcache.restrict_api with PATH_TRANSLATED ​https://github.com/php/php-src/blob/master/ext/opcache/zend_accelerator_module.c#L56. This can be different than the included file, which is returned by __FILE__.

However, $_SERVER['PATH_TRANSLATED'] is ​not always available directly.

So far, $_SERVER['SCRIPT_FILENAME'] seems to most closely approximate this, but realpath() is necessary because ​`SCRIPT_FILENAME` can be a relative path on the command line.

I would love to hear if there is a better way to get the PATH_TRANSLATED that is used.

[ayeshrajans]

Opcache is per-SAPI. This means even if we clear the opcache for CLI, it wouldn't affect other SAPIs like fpm or Apache mod_php.

One of the challenges I faced in Composer Preload plugin is warming up cache from a different SAPI. I learned that it's not possible to populate/clear opcache of fpm/etc from CLI or the other way around. So I propose that we don't even need to bother to clear opcache if the update is done by CLI (which might make the tests even more complicated).

[kirasong]

Replying to ayeshrajans:

Opcache is per-SAPI. This means even if we clear the opcache for CLI, it wouldn't affect other SAPIs like fpm or Apache mod_php.

One of the challenges I faced in Composer Preload plugin is warming up cache from a different SAPI. I learned that it's not possible to populate/clear opcache of fpm/etc from CLI or the other way around. So I propose that we don't even need to bother to clear opcache if the update is done by CLI (which might make the tests even more complicated).

That's good to know, thanks! I'm thinking that for a first pass at this, it's fine to focus on clearing whatever the current SAPI is. Then, we could open another ticket to handle async invalidation, maybe, in the case that upgrades happen via the command line, but folks need scripts to be invalidated with another SAPI?

So far, the approach I mentioned seems to be working okay on both the command line and otherwise. Hoping to get that testing done today and posted.

I haven't dug more into update-core.php and what is best to do there, so any knowledge on the workings is appreciated. Currently thinking I *might* be able to test it with WP-CLI and creating a custom package to update from.

[kirasong]

Changes to using realpath( $_SERVER['SCRIPT_FILENAME'] ) and adds additional docs.

[kirasong]

Check before calling wp_opcache_invalidate() in wp-admin/includes/update-core.php and update docs for _copy_dir() (much of the docs from 36455.6.2.diff).

[nextendweb]

@mikeschroder 36455.16.diff
Maybe there should be a local implementation of wp_opcache_invalidate => _wp_opcache_invalidate inside update-core.php like the copy_dir => _copy_dir as it would result backward opcache invalidate, which is good I think.

[kirasong]

Invalidate update-core.php before update, as it is copied separately.

[kirasong]

Verified that WP does seem to use the new update-core.php in my tests (looks to be copied ​here), so went ahead and added a check, plus invalidation of the file itself, in 36455.17.diff​.

As you mentioned, @nextendweb, it might make sense to consider porting the function to _copy_dir(), to take advantage of it in *this* upgrade. If I understand properly, the benefit would be one core version of invalidation (from x.x->5.5) earlier, correct?

If so, I'm a bit uncomfortable committing to maintaining the function in two locations indefinitely for that. Open to ideas/opinions there, for sure, though.

I think this is probably good to go in now as a first pass, but would appreciate any review/testing (and insights on automating testing), since it's easy to miss things with manual testing, and further knowledge into things I might be missing with updates is helpful.

[nextendweb]

@mikeschroder Yes, it does the invalidate when upgrading from version < 5.5 to 5.5 or higher. Maybe it is not worth the effort, I'm not sure how many people had trouble with the opcache in the past on WP upgrade.

[kirasong]

@nextendweb Yeah, it'd be great to help folks. Wonder if there's a good middle ground to avoid having too much duplicated code.

Another note: Realized that some of these functions may be used for downgrades too, so passing true for force more might make sense.

Not sure if important for a first pass, but wanted to document it here for feedback.

[nextendweb]

Another note: Realized that some of these functions may be used for downgrades too, so passing true for force more might make sense.

If I read PHP source code well, it checks if the modified timestamp is equal or not:
Line 1069 ->if (zend_get_file_handle_timestamp(file_handle, NULL) == persistent_script->timestamp) {

​https://github.com/php/php-src/blob/e9f295ac849aa72304eb6ca01c6289a928cc77dc/ext/opcache/ZendAccelerator.c

So If you downgrade, the modification time of the file will change, so force is not required.

Maybe a PHP developer should confirm it.

[kirasong]

Replying to nextendweb:

If I read PHP source code well, it checks if the modified timestamp is equal or not: <snip>

Good find, thanks! I'll test it locally to be sure, but looks like things should be good with that, then.

[kirasong]

In 48160:

Upgrade/Install: Invalidate OPcache for PHP files during updates.

When files are copied into place, check whether opcode invalidation is available and attempt to invalidate to avoid unintended behavior or fatal errors from themes, plugins, or core.

Introduces wp_opcache_invalidate() to allow safe invalidation of PHP files from opcode cache, and a filter, wp_opcache_invalidate_file to override the behavior.

Replaces the existing calls to opcache_invalidate() in the plugin and theme editors to use the new function.

Thanks to jnylen0 for porting over a patch from ClassicPress that provided much of the approach for what is being committed.

Props nigro.simone, dd32, JasWSInc, szepe.viktor, swissspidy, JanR, asalce, Garavani, pavelevap, pputzer, GregLone, benoitchantre, jadonn, doc987, kraftbj, Krstarica, jnylen0, nextendweb, williampatton, ayeshrajans, joostdevalk, stevenkussmaul, boogah, jorbin, mikeschroder.
Fixes #36455, #50354.

[kirasong]

In 48161:

Upgrade/Install: Fix Yoda condition in wp_opcache_invalidate().

Fixes linting failure due to Yoda condition in wp_opcache_invalidate() following [48160].

See #36455.

[kirasong]

In 48632:

Docs: Correct wp_opcache_invalidate_file filter docs.

Changes incorrectly named $filename variable to $filepath and adjusts wording to align with PHP docs.

See #36455.

[kirasong]

​https://make.wordpress.org/core/2020/07/27/opcode-cache-invalidation-in-wordpress-5-5/

[kirasong]

In 50475:

Upgrade/Install: Invalidate OPcache for version.php during update.

Invalidate OPcache for wp-content/version.php on update individually, since it’s copied separately.

Props stodorovic, krstarica, cenay, desrosj, pbiron, sergeybiryukov, mikeschroder.
Fixes #51686, #51281.
See #36455.

[kirasong]

In 50477:

Upgrade/Install: Invalidate OPcache for version.php during update.

Invalidate OPcache for wp-content/version.php on update individually, since it's copied separately.

Props stodorovic, krstarica, cenay, desrosj, pbiron, sergeybiryukov, mikeschroder.

Reviewed by mikeschroder, SergeyBiryukov.
Merges [50460] and [50475] to the 5.7 branch.
Fixes #51686, #51281.
See #36455.

[mark-k]

This sounds like an incomp[lete solution. Maybe I should start a new ticket but maybe keeping this context will be better.

My understanding is that opcache do not purge anything from the opcache and the best you can get is mark a file as "waste" which is what this solution does, or reset the cache.
The problem with marking files as "waste" is that they keep polluting the opcache and at some point, on servers that run a long time, you are going to get the opcache into full state. What happens at this point depends on the amount of "waste" you have, if its passes some treshold you are going to get an unsolicited reset, but a much worse case is that no reset happens and files are simply not cached any longer and the performance is being degraded with each new plugin/theme/core version without any notification to the user.

At the very least there should be a mechanism which notifies the user that it is a good time to reset the opcache and give them the tools for that.

[kirasong]

@mark-k Are you noticing that the current/new behavior is causing problems with the issues noted in this ticket?

If not, or not directly, I would suggest creating a new ticket proposing the enhancement you're talking about (with a mention / link to this ticket for any context necessary).

I think that'll give more space for conversation specifically about the new suggestion.