From 39fa2b5f9eb05c6af9bd5041abce0aad5761f622 Mon Sep 17 00:00:00 2001 From: Josh Aas Date: Tue, 30 Sep 2008 02:06:18 -0400 Subject: [PATCH] Get rid of fundamentally flawed UNIX stat cache. b=456435 r=smichaud sr=dougt --- xpcom/io/nsLocalFileUnix.cpp | 55 +++++++++++++----------------------- xpcom/io/nsLocalFileUnix.h | 8 ++---- 2 files changed, 23 insertions(+), 40 deletions(-) diff --git a/xpcom/io/nsLocalFileUnix.cpp b/xpcom/io/nsLocalFileUnix.cpp index 8ab912f09823..f298dc3c36b8 100644 --- a/xpcom/io/nsLocalFileUnix.cpp +++ b/xpcom/io/nsLocalFileUnix.cpp @@ -29,6 +29,7 @@ * Pete Collins * Paul Ashford * Fredrik Holmqvist + * Josh Aas * * Alternatively, the contents of this file may be used under the terms of * either of the GNU General Public License Version 2 or later (the "GPL"), @@ -48,7 +49,6 @@ * Implementation of nsIFile for ``Unixy'' systems. */ -// We're going to need some autoconf loving, I can just tell. #include #include #include @@ -99,13 +99,10 @@ #define FILE_STRNCMP strncmp #endif -#define VALIDATE_STAT_CACHE() \ +#define ENSURE_STAT_CACHE() \ PR_BEGIN_MACRO \ - if (!mHaveCachedStat) { \ - FillStatCache(); \ - if (!mHaveCachedStat) \ - return NSRESULT_FOR_ERRNO(); \ - } \ + if (!FillStatCache()) \ + return NSRESULT_FOR_ERRNO(); \ PR_END_MACRO #define CHECK_mPath() \ @@ -246,14 +243,12 @@ nsDirEnumeratorUnix::Close() return NS_OK; } -nsLocalFile::nsLocalFile() : - mHaveCachedStat(PR_FALSE) +nsLocalFile::nsLocalFile() { } nsLocalFile::nsLocalFile(const nsLocalFile& other) : mPath(other.mPath) - , mHaveCachedStat(PR_FALSE) { } @@ -278,25 +273,24 @@ nsLocalFile::nsLocalFileConstructor(nsISupports *outer, return inst->QueryInterface(aIID, aInstancePtr); } -nsresult +PRBool nsLocalFile::FillStatCache() { #ifdef HAVE_STAT64 if (stat64(mPath.get(), &mCachedStat) == -1) { // try lstat it may be a symlink if (lstat64(mPath.get(), &mCachedStat) == -1) { - return NSRESULT_FOR_ERRNO(); + return PR_FALSE; } } #else if (stat(mPath.get(), &mCachedStat) == -1) { // try lstat it may be a symlink if (lstat(mPath.get(), &mCachedStat) == -1) { - return NSRESULT_FOR_ERRNO(); + return PR_FALSE; } } #endif - mHaveCachedStat = PR_TRUE; - return NS_OK; + return PR_TRUE; } NS_IMETHODIMP @@ -337,7 +331,6 @@ nsLocalFile::InitWithNativePath(const nsACString &filePath) --len; mPath.SetLength(len); - InvalidateCache(); return NS_OK; } @@ -532,7 +525,6 @@ nsLocalFile::AppendRelativeNativePath(const nsACString &fragment) else mPath.Append(NS_LITERAL_CSTRING("/") + fragment); - InvalidateCache(); return NS_OK; } @@ -598,7 +590,6 @@ nsLocalFile::SetNativeLeafName(const nsACString &aLeafName) nsACString::const_iterator begin, end; LocateNativeLeafName(begin, end); mPath.Replace(begin.get() - mPath.get(), Distance(begin, end), aLeafName); - InvalidateCache(); return NS_OK; } @@ -933,20 +924,18 @@ NS_IMETHODIMP nsLocalFile::Remove(PRBool recursive) { CHECK_mPath(); + ENSURE_STAT_CACHE(); + + PRBool isSymLink; - VALIDATE_STAT_CACHE(); - PRBool isSymLink, isDir; - nsresult rv = IsSymlink(&isSymLink); if (NS_FAILED(rv)) return rv; if (!recursive && isSymLink) return NSRESULT_FOR_RETURN(unlink(mPath.get())); - - isDir = S_ISDIR(mCachedStat.st_mode); - InvalidateCache(); - if (isDir) { + + if (S_ISDIR(mCachedStat.st_mode)) { if (recursive) { nsDirEnumeratorUnix *dir = new nsDirEnumeratorUnix(); if (!dir) @@ -1008,7 +997,7 @@ nsLocalFile::SetLastModifiedTime(PRInt64 aLastModTime) int result; if (! LL_IS_ZERO(aLastModTime)) { - VALIDATE_STAT_CACHE(); + ENSURE_STAT_CACHE(); struct utimbuf ut; ut.actime = mCachedStat.st_atime; @@ -1020,7 +1009,6 @@ nsLocalFile::SetLastModifiedTime(PRInt64 aLastModTime) } else { result = utime(mPath.get(), nsnull); } - InvalidateCache(); return NSRESULT_FOR_RETURN(result); } @@ -1063,7 +1051,7 @@ NS_IMETHODIMP nsLocalFile::GetPermissions(PRUint32 *aPermissions) { NS_ENSURE_ARG(aPermissions); - VALIDATE_STAT_CACHE(); + ENSURE_STAT_CACHE(); *aPermissions = NORMALIZE_PERMS(mCachedStat.st_mode); return NS_OK; } @@ -1086,8 +1074,6 @@ nsLocalFile::SetPermissions(PRUint32 aPermissions) { CHECK_mPath(); - InvalidateCache(); - /* * Race condition here: we should use fchmod instead, there's no way to * guarantee the name still refers to the same file. @@ -1108,7 +1094,7 @@ nsLocalFile::GetFileSize(PRInt64 *aFileSize) { NS_ENSURE_ARG_POINTER(aFileSize); *aFileSize = LL_ZERO; - VALIDATE_STAT_CACHE(); + ENSURE_STAT_CACHE(); #if defined(VMS) /* Only two record formats can report correct file content size */ @@ -1136,7 +1122,6 @@ nsLocalFile::SetFileSize(PRInt64 aFileSize) PRInt32 size; LL_L2I(size, aFileSize); /* XXX truncate64? */ - InvalidateCache(); if (truncate(mPath.get(), (off_t)size) == -1) return NSRESULT_FOR_ERRNO(); return NS_OK; @@ -1385,7 +1370,7 @@ nsLocalFile::IsDirectory(PRBool *_retval) { NS_ENSURE_ARG_POINTER(_retval); *_retval = PR_FALSE; - VALIDATE_STAT_CACHE(); + ENSURE_STAT_CACHE(); *_retval = S_ISDIR(mCachedStat.st_mode); return NS_OK; } @@ -1395,7 +1380,7 @@ nsLocalFile::IsFile(PRBool *_retval) { NS_ENSURE_ARG_POINTER(_retval); *_retval = PR_FALSE; - VALIDATE_STAT_CACHE(); + ENSURE_STAT_CACHE(); *_retval = S_ISREG(mCachedStat.st_mode); return NS_OK; } @@ -1426,7 +1411,7 @@ NS_IMETHODIMP nsLocalFile::IsSpecial(PRBool *_retval) { NS_ENSURE_ARG_POINTER(_retval); - VALIDATE_STAT_CACHE(); + ENSURE_STAT_CACHE(); *_retval = S_ISCHR(mCachedStat.st_mode) || S_ISBLK(mCachedStat.st_mode) || #ifdef S_ISSOCK diff --git a/xpcom/io/nsLocalFileUnix.h b/xpcom/io/nsLocalFileUnix.h index c51f2d9d041b..a124b081fc71 100644 --- a/xpcom/io/nsLocalFileUnix.h +++ b/xpcom/io/nsLocalFileUnix.h @@ -113,13 +113,14 @@ private: ~nsLocalFile() {} protected: +// This stat cache holds the *last stat* - it does not invalidate. +// Call "FillStatCache" whenever you want to stat our file. #ifdef HAVE_STAT64 struct stat64 mCachedStat; #else struct stat mCachedStat; #endif nsCString mPath; - PRPackedBool mHaveCachedStat; void LocateNativeLeafName(nsACString::const_iterator &, nsACString::const_iterator &); @@ -130,10 +131,7 @@ protected: const nsACString &newName, nsACString &_retval); - void InvalidateCache() { - mHaveCachedStat = PR_FALSE; - } - nsresult FillStatCache(); + PRBool FillStatCache(); nsresult CreateAndKeepOpen(PRUint32 type, PRIntn flags, PRUint32 permissions, PRFileDesc **_retval);