Permalink
Browse files

Merge pull request #181 from 0x20h/1660-cache-locking

backporting fix for #1660 to branch 1.2
  • Loading branch information...
2 parents 0ddc5f1 + b5db77e commit e3b4a35a65f1bef0e87f10804a018284adeef5f7 @lorenzo lorenzo committed Aug 23, 2011
Showing with 17 additions and 6 deletions.
  1. +17 −6 cake/libs/cache/file.php
View
@@ -139,13 +139,24 @@ function write($key, &$data, $duration) {
}
}
- if ($this->settings['lock']) {
- $this->__File->lock = true;
- }
$expires = time() + $duration;
$contents = $expires . $lineBreak . $data . $lineBreak;
- $success = $this->__File->write($contents);
- $this->__File->close();
+
+ if (!$handle = fopen($this->__File->path, 'c')) {
@davidpersson

davidpersson Aug 23, 2011

Contributor

The c flag is available >= PHP 5.2.6.

@lorenzo

lorenzo Aug 23, 2011

Owner

that's very unfortunate, thanks for pointing this out!

@davidpersson

davidpersson Aug 24, 2011

Contributor

Yeah, but maybe file_put_contents() in combination with LOCK_EX is an alternative. Although I'm not 100% sure about the inner workings of that function (i.e. if/when it does flushing).

@jrbasso

jrbasso Aug 24, 2011

Member

file_put_contents is PHP5+. One option to PHP4 is open the file with 'w' using locks. Is not the best option, but is available to the case.

@markstory

markstory Aug 24, 2011

Owner

Lame, I missed that in the original commit to 1.3. I guess, we'll have to use 'w' or 'a' for the time being, and accept that there are no atomic operations on files for now.

@markstory

markstory Aug 24, 2011

Owner

I quickly tried using fopen() with 'a', and it worked from the basic tests I ran. I can commit the change to 1.2 and 1.3 tomorrow if there aren't any objections.

@lorenzo

lorenzo Aug 24, 2011

Owner

I think it's fine, we can use 'c' for 2.0

@0x20h

0x20h Aug 30, 2011

Contributor

Argh, sorry for not checking this.
I also think 'a' is ok as the file gets truncated first.
Thanks david for checking this out, thanks Mark for the fixes :-)

@markstory

markstory Aug 30, 2011

Owner

Its currently 'a' in 1.x and 'c' in 2.0. I think that's the best we can do for now.

@0x20h

0x20h Aug 31, 2011

Contributor

I think both modes are equivalent regarding the problem.
The point is that the file must not be changed before the lock is obtained. This is true for 'a' and/or 'c' ('w' did truncate before obtaining the lock).
First thing I thought on 'a' was that the file pointer would have to be reset after truncating, 'c' would set the pointer to the beginning. However, I just found a note on fseek that states that in 'a' mode any write will append to the end of the file, regardless of the pointer (see php-docs). So, I think in this specific case there is no difference between the two modes.

+ return false;
+ }
+
+ if ($this->settings['lock']) {
+ flock($handle, LOCK_EX);
+ }
+
+ $success = ftruncate($handle, 0) && fwrite($handle, $contents) && fflush($handle);
+
+ if ($this->settings['lock']) {
+ flock($handle, LOCK_UN);
+ }
+
+ fclose($handle);
return $success;
}
/**
@@ -265,4 +276,4 @@ function __active() {
return true;
}
}
-?>
+?>

0 comments on commit e3b4a35

Please sign in to comment.