php.net |  support |  documentation |  report a bug |  advanced search |  search howto |  statistics |  random bug |  login
Bug #56080 cache might contain invalid content
Submitted: 2004-05-30 07:40 UTC Modified: 2004-09-15 11:13 UTC
From: swen dot thuemmler at telefonica dot de Assigned: rasmus (profile)
Status: Closed Package: APC (PECL)
PHP Version: Irrelevant OS: Solaris 2.8
Private report: No CVE-ID: None
View Add Comment Developer Edit
Anyone can comment on a bug. Have a simpler test case? Does it work for you on a different platform? Let us know!
Just going to say 'Me too!'? Don't clutter the database with that please !
Your email address:
MUST BE VALID
Solve the problem:
31 - 5 = ?
Subscribe to this entry?

 
 [2004-05-30 07:40 UTC] swen dot thuemmler at telefonica dot de
Description:
------------
APC uses the inode number as a cache key and uses mtime to check,
wether the cache content is still valid.
I see a possible problem with this: apc does not know when a file gets
deleted. So a file might get deleted and later a new file created with
the same inode and possibly a smaller mtime (Smarty for instance
creates its php files with the mtime of the originating template). Now
a request for this file will get the old content of the deleted file, not
the content of the new one.
A possible solution: set key.mtime to the max of (mtime, ctime), see
enclosed patch.
Comments?

diff -u -r3.55 apc_cache.c
--- apc_cache.c 29 Mar 2004 17:18:11 -0000      3.55
+++ apc_cache.c 30 May 2004 11:47:10 -0000
@@ -346,7 +346,7 @@
 
     key->device = buf.st_dev;
     key->inode  = buf.st_ino;
-    key->mtime  = buf.st_mtime;
+    key->mtime  = (buf.st_ctime > buf.st_mtime) ? buf.st_ctime: buf.st_mtime;
     return 1;
 }
 /* }}} */



Patches

Add a Patch

Pull Requests

Add a Pull Request

History

AllCommentsChangesGit/SVN commitsRelated reports
 [2004-08-17 09:05 UTC] is at eseco dot de
I tried the latest APC release package (2.0.4) and ran into that problem. Smarty had a lot of problems and I had to remove apc. 

Reading your problem report, I checked the latest cvs apc_cache.c file and was wondering, whether your patch is applied already. But it isn't... What does this mean? Is your patch not fixing the problem or has it been solved differently?

Would be great if one of the developers could provide a statement on this issue.
 [2004-09-07 00:47 UTC] rasmus@php.net
Smarty has since gotten rid of this rather silly concept of creating files and resetting the mtime on them.  But checking the ctime is still not a bad idea.
 [2004-09-07 02:27 UTC] rasmus@php.net
Committed suggested patch to CVS
 [2004-09-14 13:33 UTC] rasmus@php.net
I have decided to revert this change.  It really is bogus for systems to create files with mtimes older than previous versions of the file.  The big drawback in using the ctime here is that it breaks cache warmup strategies where you write the new page to a temporary file, hit it once to cache it, and then rename it to the final destination.  mtime doesn't change on the rename, but ctime would and thereby defeat the whole purpose of the cache warmup.
 [2004-09-15 03:54 UTC] swen dot thuemmler at telefonica dot de
Hmm, this still leaves the case when a file gets deleted and
a new file with same inode but different - possibly smaller - mtime gets created. APC would not detect this case. Simple but IMHO effetive solution: check whether mtime changes, not only whether it increases. Comments?

Regards, Swen

diff -u -r3.71 apc_cache.c
--- apc_cache.c 14 Sep 2004 17:34:28 -0000      3.71
+++ apc_cache.c 15 Sep 2004 07:54:09 -0000
@@ -331,7 +331,7 @@
     while (*slot) {
         if (key_equals((*slot)->key.data.file, key.data.file)) {
             /* If existing slot for the same device+inode is older, remove it and insert the new version */
-            if ((*slot)->key.mtime < key.mtime) {
+            if ((*slot)->key.mtime != key.mtime) {
                 remove_slot(cache, slot);
                 break;
             }
@@ -412,7 +412,7 @@
 
     while (*slot) {
         if (key_equals((*slot)->key.data.file, key.data.file)) {
-            if ((*slot)->key.mtime < key.mtime) {
+            if ((*slot)->key.mtime != key.mtime) {
                 remove_slot(cache, slot);
                 break;
             }
 [2004-09-15 10:23 UTC] rasmus@php.net
So you are saying that the inodes will roll over once and the "new" file that gets that same inode could have an older mtime than the previous file that held that inode?  What sort of messed up system is that?  I repeat, systems that create files with old mtimes are bogus and should be fixed.  The Smarty guys realized that and got rid of that braindeadness.

And no, this won't work.  If a file is changed rapidly your version will mess up.  Say the server is being hit hard and you change the file 3 times quickly.  Call them versions A,B and C of that file.  10 requests come in at version A, see that it isn't cached and start to compile to opcodes.  While this is happening the file is changed and 10 more requests come in on version B, changed again and 10 more on version C.  Can you with absolute certainty assure me that all of the version C requests will finish after all of the version A and version B requests?  If you can't, then you could end up with version A or version B of the file in the cache after those 3 rapid file changes instead of version C which would be the correct version to be in the cache.
 [2004-09-15 10:55 UTC] swen dot thuemmler at telefonica dot de
Well, until last week, smarty used to change the mtime of the compiled templates. This has changed in the latest version, which avoids this special problem (but leads to other "interesting" effects if you use a content distribution system which uses mtimes (we are using rsync to replicate the source tree). If you have a template which is compiled on the frontend after the template on the master is changed but before it is distributed to the client, the change on the client will not be visible. But I can live with that.

And your example with the rapid file change: this problem stays the same whether you use '<' or '!=', so I think using
'!=' should be ok. What do you think?

--Swen
 [2004-09-15 11:05 UTC] rasmus@php.net
No, when you use < the problem is completely fixed.  In my example say just in the extreme case that all the C requests finish first which could actually happen quite easily if the size of the script was reduced dramatically in one of those changes.  So the C requests have finished and the file has been cached with C's mtime which is greater than A and B's.  Now an A request finishes and wants to store its set of opcodes in the cache, but because A's mtime is < than C's mtime this attempt to cache these older opcodes is correctly ignored and the C opcodes stay cached.  So I am not sure how you can say the problem stays the same.
 [2004-09-15 11:13 UTC] swen dot thuemmler at telefonica dot de
Ahh, now I see. Of course you're right. I'll upgrade Smarty and revert my change.

Thanks, Swen
 
PHP Copyright © 2001-2024 The PHP Group
All rights reserved.
Last updated: Thu Mar 28 14:01:29 2024 UTC