Hi,

Yes -- the original reason for dev/inode on Unix instead of filename was to reduce memory consumed in the case of a large # of symlinks or hardlinks to the same file. This was the case for AOL's digitalcity.com back in 1999. For better or worse, the "AOL" in AOLserver means AOL was used as the model and we never had an example of dynamically creating files to be returned by fastpath (this isn't 100% true -- see below).

Now, had we known 10 years ago that inodes could be rapidly reused as John pointed out, we would have never written the code to use dev/ inode as opposed to filename keys. It simply cannot be strictly correct given the non-atomic nature of the stat() before the open(). It may have been available as an option, but certainly off by default and likely undocumented.

So, here's what I'd suggest:

-- Cache by filename key should be the default. This is technically the correct fix to enable temporary, uniquely named files, to be returned via ns_returnfile. -- John's "grace period" code is a clever optimization if fastpath is being used in this way and could also be an option, default off.

To clarify one point: There is no technical solution to creating temp files with the same name and avoiding the race condition without additional synchronization. This is expecting a "private per-thread view" of the global filesystem namespace which isn't strictly possible even if some sort of heuristics (such as the grace period trick above) nearly eliminate the possibility in practice. The Unix file system has always had atomic features which have traditionally been used to solve for race conditions using the uniqueness of filenames (via O_EXCL open/creat flag and atomic rename). It's reasonable to assume programmers understand and leverage that fact; it's not reasonable to attempt to solve for cases they don't. You can see some careful tempfile creation code in Ns_GetTemp in fd.c if interested.



Finally, for those still awake and sober: The "not 100% true" comment above was that there was one solution at AOL where ADP code would on- demand create new ADP code. The idea was to do some "hard" work (query database, fetch from remote, etc.) while leaving other ADP code to do "easy" stuff on the fly (select a promo, etc.). AOLserver now has direct support for this with it's new caching stuff in 4.5 but back in '99 that didn't exists. So, technically this was a case where we dynamically created code which was later read by ADP (which had the same dev/inode cache stuff as fastpath). However, this was done carefully:

-- Tcl-level mutex/condition variables to ensure only one thread did the "hard" work even if several were interested in the result
-- Careful write to a non .adp extension, unique temp file
-- Atomic rename in place when ready

It was a combination of traditional atomic Unix filesystem semantics and newer thread synchronization at the Tcl level used to avoid ever getting some mutant result.

BTW: If you're still interested and awake, check the ADP cache/parse code -- it has some code to detect modification in place during parse even though at AOL we would have never allowed that possibility (we'd use the atomic rename technique describe above). That's one case were code was carefully written to mitigate poor programming practice we ourselves would never have allowed (a programmer who had written such code would have ended up in my office for an uncomfortable chat). I seem to remember trying for a very long time to exercise the fail cases as things rarely would trip up even under high load but we made sure the code was in there anyway because it was closer to correct and blindly writing a file in place is sadly quite common.

-Jim






On Aug 21, 2008, at 3:24 PM, Rusty Brooks wrote:

I don't have any opinion on the fix, but I think the actual objection to using the filename in the fix is that this would cause hard links to files, which are for all intents and purposes The Same File, to be considered different files by fastpath. (Hard links have different names, but the same inode)

Rusty

Titi Alailima wrote:
I agree that John's patch is worth doing. It satisfies both his requirements and the stated design goals of fastpath. The remaining issue is whether something called "ns_returnfile" which takes a pathname as a parameter should have some guarantee that you will return what at least at some point was the contents of a file with that pathname. It's perfectly acceptable in dealing with caching systems that the cached value could be out of sync, but not that the cached value could be for something entirely different from what you were looking for. Even with the mtime fix there's no guarantee that systems which muck around with mtime (such as tar) won't cause separate files to collide. For a contrived example: 1. tar xf foo.tar (creating two files "a" and "b" with the same size and same mtime)
2. ns_returnfile b
3. Delete files "a" and "b"
4. tar xf foo.tar
5. ns_returnfile b (this could return the contents of "a" because the inode was reused) I don't think this example violates any of the stated principles of using ns_returnfile for only "static" data. Both "a" and "b" could have completely stable contents and due to some minor issue of system administration (for example) their inodes could end up swapped and the cache poisoned. So I think we need both fixes, one to eliminate caching unless a certain criterion of "static-ness" has been met, and the other to prevent the cache from returning completely unrelated data. Other caveats about ns_returnfile use still apply, and the documentation should reflect them. Now the only people this wouldn't satisfy are those who are concerned about pathnames taking up space in the cache or slowing it down. The option has been suggested to make pathname inclusion optional, though I would advise against it unless the configuration option is named in such a way as to indicate its "unsafe"-ness.
Titi Ala'ilima
Lead Architect
MedTouch LLC
1100 Massachusetts Avenue
Cambridge, MA 02138
617.621.8670 x309
-----Original Message-----
From: AOLserver Discussion [mailto:[EMAIL PROTECTED] On
Behalf Of Tom Jackson
Sent: Thursday, August 21, 2008 12:25 PM
To: AOLSERVER@LISTSERV.AOL.COM
Subject: Re: [AOLSERVER] Data "corruption" with fastpath caching

On Thu, 2008-08-21 at 11:14 -0400, Dossy Shiobara wrote:
4) I see the simplest (best?) solution here being a configurable
parameter that controls fastpath's cache key generation.  As Jim
points
out, one can quickly test whether this would solve the problem at
hand
by temporarily #define'ing _WIN32 in the appropriate place. If this
proves successful, we change it from using #ifdef's to regular if()
statements and define a new configuration parameter.  End of
discussion.

I have responded twice to John's newest patch idea, which is a one line
patch. It appears to completely eliminate any problem with cache
poisoning. It is simple, it doesn't change the semantics of the command or anything else. It simply works around a known limitation of the stat
mtime granularity.

The only security issue that was exposed was the misuse of
ns_returnfile. All of the data put into cache were entirely under the
control of the AOLserver process. The developer / maintainer of that
process is responsible for everything the process does. ns_returnfile
is
an inherently dangerous API, there is no handholding involved. You have
to understand what it is doing and why it exists.

In fact, John even pointed out that the original code which wrote out the contents of the file reused the same name over and over. Assuming
that you can know that the contents of a file have not changed just
because it has the same name, same mtime and same size is an invalid
assumption, it will always be invalid. All caches have the same
limitation. By definition they are not in sync with the true copy.
Anyone who uses a cache needs to understand this.

So, this is important, John is not interested in the cache, he actually wants to avoid the cache. So talking about how stuff is stored in the
cache, and under what key, is unimportant for John. He wants to keep
his
newly created file from ever getting into the cache.

And this is where he has a point, a very good one. Why put newly
created
files into a cache, if the point of the cache is to handle static
files?
We can wait for evidence that it is static. In this case, we can wait until it is a few seconds old, at least. John's patch does exactly this
and nothing more. It is actually a very ingenious change.


There is no difference between the inode and the filename under unix.
Both offer equal opportunity to screw up due to a race condition. It
can
still happen even in the patched ns_returnfile. Jim mentioned this.
After a file is stat'ed, the open might find a different (maybe
truncated) file. There is no guarantee that you won't get something
else, especially if you have multiple processes/threads creating files
in an non-synchronized way. It is not part of ns_returnfile to
guarantee
that the contents/age of a file remains unchanged during the course of
execution, and when you throw in an external process it is nearly
impossible to come up with any code which can provide that guarantee.
If
data integrity is really important to you, don't try to provide it
using
named files as temporary storage.

tom jackson


--
AOLserver - http://www.aolserver.com/

To Remove yourself from this list, simply send an email to
<[EMAIL PROTECTED]> with the
body of "SIGNOFF AOLSERVER" in the email message. You can leave the
Subject: field of your email blank.
--
AOLserver - http://www.aolserver.com/
To Remove yourself from this list, simply send an email to <[EMAIL PROTECTED] > with the body of "SIGNOFF AOLSERVER" in the email message. You can leave the Subject: field of your email blank.


--
AOLserver - http://www.aolserver.com/

To Remove yourself from this list, simply send an email to <[EMAIL PROTECTED] > with the body of "SIGNOFF AOLSERVER" in the email message. You can leave the Subject: field of your email blank.


--
AOLserver - http://www.aolserver.com/

To Remove yourself from this list, simply send an email to <[EMAIL PROTECTED]> 
with the
body of "SIGNOFF AOLSERVER" in the email message. You can leave the Subject: 
field of your email blank.

Reply via email to