WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 5 weeks ago

#36455 new enhancement

Call opcache_reset() after plug-in, theme or core update

Reported by: nigro.simone Owned by:
Milestone: 5.0 Priority: normal
Severity: normal Version:
Component: Upgrade/Install Keywords: has-patch needs-testing
Focuses: Cc:

Description

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?

Attachments (5)

36455.diff (1.2 KB) - added by asalce 18 months ago.
36455.2.diff (1.5 KB) - added by asalce 18 months ago.
36455.3.diff (1.5 KB) - added by asalce 18 months ago.
36455.4.diff (1.5 KB) - added by asalce 18 months ago.
Fixed a typo. No more coding at 2AM for me! :)
36455.5.diff (930 bytes) - added by benoitchantre 3 months ago.

Download all attachments as: .zip

Change History (49)

#1 @dd32
2 years ago

  • Focuses performance removed
  • Keywords needs-patch 2nd-opinion added
  • Version 4.4.2 deleted

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.

#2 @JasWSInc
2 years ago

+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.

#3 @szepe.viktor
2 years ago

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

This ticket was mentioned in Slack in #core by swissspidy. View the logs.


19 months ago

#5 @swissspidy
18 months ago

  • Milestone changed from Awaiting Review to 4.8
  • Type changed from feature request to enhancement

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.

#6 follow-up: @JanR
18 months ago

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/

#7 in reply to: ↑ 6 @szepe.viktor
18 months ago

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.

#8 @JasWSInc
18 months ago

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
18 months ago

#9 follow-up: @asalce
18 months ago

  • Keywords has-patch needs-testing added; needs-patch removed

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.

#10 in reply to: ↑ 9 ; follow-up: @JanR
18 months ago

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.

#11 in reply to: ↑ 10 @asalce
18 months ago

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
18 months ago

@asalce
18 months ago

#12 @asalce
18 months ago

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.

Last edited 18 months ago by asalce (previous) (diff)

@asalce
18 months ago

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

#13 @Garavani
16 months ago

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.

#14 @pavelevap
15 months ago

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

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


14 months ago

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


14 months ago

#17 @jbpaul17
14 months ago

  • Milestone changed from 4.8 to 4.8.1

Punting to 4.8.1 per today's bug scrub.

#18 follow-up: @mikeschroder
14 months ago

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()?

#19 @szepe.viktor
14 months ago

<?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

Last edited 14 months ago by szepe.viktor (previous) (diff)

#20 @szepe.viktor
14 months ago

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.

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


12 months ago

#22 @jbpaul17
12 months ago

  • Milestone changed from 4.8.1 to 4.9

Punting to 4.9 per today's bug scrub.

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


9 months ago

#24 @jbpaul17
9 months ago

  • Milestone changed from 4.9 to Future Release

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

#25 @pputzer
5 months ago

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.

This ticket was mentioned in Slack in #core by benoitchantre. View the logs.


4 months ago

This ticket was mentioned in Slack in #core by jeffpaul. View the logs.


4 months ago

This ticket was mentioned in Slack in #core-php by benoitchantre. View the logs.


4 months ago

This ticket was mentioned in Slack in #core-php by benoitchantre. View the logs.


3 months ago

#30 @mikeschroder
3 months ago

  • Keywords needs-patch added; 2nd-opinion has-patch needs-testing removed

#31 @GregLone
3 months ago

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.

This ticket was mentioned in Slack in #core by benoitchantre. View the logs.


3 months ago

#33 in reply to: ↑ 18 @benoitchantre
3 months ago

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.

#34 @benoitchantre
3 months ago

  • Keywords has-patch needs-testing added; needs-patch removed

This ticket was mentioned in Slack in #core by audrasjb. View the logs.


3 months ago

This ticket was mentioned in Slack in #hosting-community by dd32. View the logs.


3 months ago

This ticket was mentioned in Slack in #hosting-community by mike. View the logs.


3 months ago

#38 @SergeyBiryukov
3 months ago

  • Milestone changed from Future Release to 5.0

#39 follow-up: @jadonn
3 months ago

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.

#40 in reply to: ↑ 39 @pputzer
3 months ago

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).

#41 follow-up: @GregLone
2 months ago

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;
}

#42 in reply to: ↑ 41 @benoitchantre
2 months ago

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?

#43 @GregLone
2 months ago

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

#44 @doc987
5 weeks ago

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).

Note: See TracTickets for help on using tickets.