Cache Functions Suggestions

[eluser]Elliot Haughin[/eluser]
Ah, good catch... I've done some work on this, and it turns out that the way around this is to test on the flock with an extra LOCK_NB passed to it...

Output.php line 299:

Replace:

Code:

if ( ! $fp = @fopen($cache_path, 'wb'))
        {
            log_message('error', "Unable to write cache file: ".$cache_path);
            return;
        }
        
        $expire = time() + ($this->cache_expiration * 60);
        
        flock($fp, LOCK_EX);
        fwrite($fp, $expire.'TS--->'.$output);
        flock($fp, LOCK_UN);
        fclose($fp);
        @chmod($cache_path, 0777);

        log_message('debug', "Cache file written: ".$cache_path);

Code:

if ( ! $fp = @fopen($cache_path, 'wb'))
        {
            log_message('error', "Unable to write cache file: ".$cache_path);
            return;
        }
        else
        {
            $expire = time() + ($this->cache_expiration * 60);

            if ( flock($fp, LOCK_EX | LOCK_NB) )
            {
                fwrite($fp, $expire.'TS--->'.$output);
                flock($fp, LOCK_UN);

                log_message('debug', "Cache file written: ".$cache_path);
            }
            else
            {
                log_message('debug', "Cache file not written (due to locking): ".$cache_path);
                return;
            }

            fclose($fp);
            @chmod($cache_path, 0777);
        }

I'll forward this little gem on to Derek Allard... I'm sure he'll notice this anyway, but nice catch Rick, hopefully this solution should solve this situation.

[eluser]Rick Jolly[/eluser]
Right on Elliot. That improvement will also prevent users from hanging indefinitely if something terrible sould happen and the lock is never released. That would be an incredibly uncommon event, but I read about that issue in the EE forum. In that case, I think the sys admin had to reboot the server.

[eluser]Elliot Haughin[/eluser]
Well there we go, problem solved

Hopefully this will go in 1.6.2 release.

[eluser]section31[/eluser]
hm, didn't know my thread got big. I'm glad something good came out of it.

[eluser]Paul Burdick[/eluser]
Rick & Elliot, thanks for putting some serious effort into this. As Rick mentions, there was an issue with a lock in ExpressionEngine never being released on someone's server causing a build up, so we were planning on revisiting all of the flock() usage in both CodeIgniter and ExpressionEngine during this half of 2008. Looking at your fix, you are using LOCK_NB, which is not available on Windows. And, if anything, the comments for fopen() and flock() on PHP.net seem to indicate further problems with locking on other servers too.

So, I was wondering, as you obviously have put some thought into this, have you considered an approach that might work on all servers for cache writing? My thoughts were towards creating a temporary file for writing the cache and using the filesystem (perhaps copy()) for replacing the current cached copy, but I have not been able to give it any serious thinking or testing yet.

[eluser]section31[/eluser]
Isn't that what I initially suggested?

[eluser]Paul Burdick[/eluser]
Not exactly what I had in mind, no.

[eluser]Rick Jolly[/eluser]
Hi Paul. Thank you for responding. I think there are two routes to take:

1) prevent cache file corruption with flock
2) detect cache file corruption from writes during reading

Prevent corruption with flock
I'm no expert, but I haven't come across a reliable way to solve the flock issues. Adding the non-blocking lock (LOCK_NB) seems to be the ideal solution - if it were supported by all operating systems...
[quote author="Paul Burdick" date="1203108520"]...Looking at your fix, you are using LOCK_NB, which is only available on Windows...[/quote]
PHP.net claims that it isn't supported by windows.

Still, as long as the non-blocking lock has some support, it seems worth including. Maybe first detect the OS and add the non-blocking lock if supported.

The locks work on the OS level, and every hack I've seen to replace flock doesn't work. Using rename/copy to replace the cache file would corrupt the file for those users simultaneously reading the file - I've tested that to be sure.

Detect corruption from writes during reading
So if flock is out, and you can't prevent cache file corruption for readers, then the next best thing is to detect corruption. In the event the cache has been updated during a read, then simply don't use the cache. To detect corruption, a hash, checksum, or a simple length of the original data is included in the cache file when writing. When the cache file is read, that check (hash, checksum, or length) is extracted from the file and compared with the same check on the actual data. Of course, the drawback is that there is overhead involved.

[eluser]Paul Burdick[/eluser]
Sorry, I misspoke about Windows, I meant "not available on Windows" Replied before breakfast was digested.

I was really hoping that the copy() by going through the filesystem and simply modifying something like a file pointer would not cause file corruption during reading. How unfortunate that it does not. Tedious really as all modern databases have the read/write problem transparently taken care of.

flock() is not out, it just seems like it is an incomplete solution and you know how we love the silver bullet. Checking for data corruption does not really strike me as the most elegant solution either. Your solution solves the problem for non-Windows users, it seems, so I will have Allard put it into SVN. Thanks.

[eluser]Elliot Haughin[/eluser]
Right, how about this for a little theory?

Code:

$lockfile = 'cache.lock';
$modified = filemtime($lockfile);

$now = time();

$diff = $now - $modified;

// Have we made our own 'lock' yet?
// Or is there one there that's been hanging around?
if (!file_exists($lockfile) || $diff > 10)
{
     // Write a little 'do not disturb sign and
     // put it on the door :)

     $lockhandle = fopen($filename, 'x+');  
     fwrite($lockhandle, "");
     fclose($lockhandle);


     // NORMAL WRITING OF CACHE FILE HERE


     unlink($lockfile);
}

It's a 'simple' solution, but the logic is pretty good, and it does a decent job.
This isn't tested, but I hope it gives you a clear picture of what I'm thinking.

It should be PHP4/5 and all OS's too...

You can even use this method to protect from read corruption too...

if ( cache file exists and lock doesn't)
then get it from cache
else
render it like normal.