WP Super Cache “needs-rebuild” code is broken

In recent versions of WP Super Cache, including the current version 0.90, the “experimental” “needs-rebuild” code doesn’t work properly.

What happens is that prune_super_cache() is called for the permalink directory. That function now has a loop that calls down to sc_garbage_collection() repeatedly, stopping only when sc_garbage_collection() doesn’t make any changes or has been run 200 times.

If sc_garbage_collection() makes any change, prune_super_cache() always calls sc_garbage_collection() again.

So what happens is that the first call to sc_garbage_collection() renames “index.html” to “index.html.needs-rebuild”. Then sc_garbage_collection() is immediately called again, notices the “index.html.needs-rebuild” file, and deletes it due to this code:

if( $cache_rebuild_files && substr( $directory, -14 ) != '.needs-rebuild' ) {
	if( @rename($directory, $directory . '.needs-rebuild') ) {
		@touch( $directory . '.needs-rebuild' );
		$gc_file_counter++;
	}
} else {
	@unlink( $directory );
	$gc_file_counter++;
}

That isn’t right: it shouldn’t delete a fresh “index.html.needs-rebuild” file. The idea is to keep that file around for at least 30 seconds so that it can be renamed back to “index.html” if necessary.

The simple patch for this is at:

https://www.tigertech.net/patches/wp-supercache-20090128142133.patch

However, the whole way prune_super_cache() is working seems odd. Couldn’t it be coded so that sc_garbage_collection() doesn’t need to always be called one unnecessary extra time after it’s finished working on every file in the directory? The sc_garbage_collection() function is not particularly lightweight; it opens and scans directories recursively, and calling it unnecessarily should be avoided.

I don’t quite understand why sc_garbage_collection() has a limit of 100 files, but then prune_super_cache() calls it up to 200 times. Couldn’t sc_garbage_collection() just have a limit of 20,000 files?

Also, I know this is nit-picky, but this stands out as ugly:

while( $c < 200 )  {
    ...
    if( $gc_file_counter == 0 )
        $c = 200;
}

That kind of thing is just asking for trouble if the constants get out of sync — it should be:

while( $c < 200 )  {
    ...
    if( $gc_file_counter == 0 )
        break;
}

If you’d like me to attempt a more general patch to clean all this up, just let me know and I’ll give it a shot.

Thanks!