pluginkollektiv / cachify

Smart but efficient cache solution for WordPress. Use DB, HDD, APC or Memcached for storing your blog pages. Make WordPress faster!
https://wordpress.org/plugins/cachify/
GNU General Public License v2.0
99 stars 31 forks source link

Error in HDD mode while flushing the cache #310

Open Zodiac1978 opened 1 month ago

Zodiac1978 commented 1 month ago

I got an error message via mail:

WordPress-Version 6.6.2 Aktives Theme: Twentyfourteen Child (Version 1.0.5) Aktuelles Plugin: Cachify (Version 2.3.2) PHP-Version 8.1.30

Fehler-Details
==============
Ein Fehler vom Typ E_ERROR wurde in der Zeile 228 der Datei /example/wp-content/plugins/cachify/inc/class-cachify-hdd.php
verursacht. Fehlermeldung: Uncaught TypeError: array_diff(): Argument #1 ($array) must be of type array, bool given in
/example/wp-content/plugins/cachify/inc/class-cachify-hdd.php:228
Stack trace: #0 /example/wp-content/plugins/cachify/inc/class-cachify-hdd.php(228): array_diff()
#1 /example/wp-content/plugins/cachify/inc/class-cachify-hdd.php(243): Cachify_HDD::_clear_dir()
#2 /example/wp-content/plugins/cachify/inc/class-cachify-hdd.php(243): Cachify_HDD::_clear_dir()
#3 /example/wp-content/plugins/cachify/inc/class-cachify-hdd.php(243): Cachify_HDD::_clear_dir()
#4 /example/wp-content/plugins/cachify/inc/class-cachify-hdd.php(243): Cachify_HDD::_clear_dir()
#5 /example/wp-content/plugins/cachify/inc/class-cachify-hdd.php(243): Cachify_HDD::_clear_dir()
#6 /example/wp-content/plugins/cachify/inc/class-cachify-hdd.php(103): Cachify_HDD::_clear_dir()
#7 /example/wp-content/plugins/cachify/inc/class-cachify.php(1298): Cachify_HDD::clear_cache()
#8 /example/wp-content/plugins/cachify/inc/class-cachify.php(936): Cachify::flush_total_cache()
#9 /example/wp-includes/class-wp-hook.php(326): Cachify::publish_post_types()
#10 /example/wp-includes/class-wp-hook.php(348): WP_Hook->apply_filters()
#11 /example/wp-includes/plugin.php(517): WP_Hook->do_action()
#12 /example/wp-includes/post.php(5579): do_action()
#13 /example/wp-includes/post.php(4781): wp_transition_post_status()
#14 /example/wp-includes/post.php(4993): wp_insert_post()
#15 /example/wp-admin/includes/post.php(453): wp_update_post()
#16 /example/wp-admin/post.php(227): edit_post()
#17 {main}  thrown 

This is the code:

$objects = array_diff(
            scandir( $dir ),
            array( '..', '.' )
        );

https://github.com/pluginkollektiv/cachify/blob/develop/inc/class-cachify-hdd.php#L244C3-L248C5

It looks like scandir is returning false (maybe just temporarily while updating something else) and therefore the array_diff throws an error because it is expecting an array.

Maybe we can make this more robust?

stklcode commented 1 month ago

The question is, what would we expect in such case?

scandir fails, in other words we cannot determine the contents of a (sub)directory. Guess it’s likely to expect that we cannot delete the directory, if we just ignore the error. Probably some kind of permissions issue or the directory being deleted in parallel (with really bad timing, given there is only one line of code between check and scan).

Crashing prevents flushing the cache, leaving some half-flushed directory structure back on disk. So ... double check for existence and continue if the directory really does not exist anymore? Or log the error, propagate the negative result through e.g. return values and try to clean up everything else 🤔

Logic has not changed for years, so I guess no reason to rush into it now. But we should revisit the HDD access anyway and probably move some logic to WP_Filesystem if reasonable.