Bug 1396366: Make sure the URLPreloader cache is only written once. r?erahm
MozReview-Commit-ID: FA1BPQ5c6nP
--- a/js/xpconnect/loader/ScriptPreloader.cpp
+++ b/js/xpconnect/loader/ScriptPreloader.cpp
@@ -596,18 +596,16 @@ ScriptPreloader::PrepareCacheWrite()
//
// - A block of XDR data for the encoded scripts, with each script's data at
// an offset from the start of the block, as specified above.
Result<Ok, nsresult>
ScriptPreloader::WriteCache()
{
MOZ_ASSERT(!NS_IsMainThread());
- Unused << URLPreloader::GetSingleton().WriteCache();
-
if (!mDataPrepared && !mSaveComplete) {
MOZ_ASSERT(!mBlockedOnSyncDispatch);
mBlockedOnSyncDispatch = true;
MonitorAutoUnlock mau(mSaveMonitor);
NS_DispatchToMainThread(
NewRunnableMethod("ScriptPreloader::PrepareCacheWrite",
@@ -690,17 +688,20 @@ ScriptPreloader::Run()
// Ideally wait about 10 seconds before saving, to avoid unnecessary IO
// during early startup. But only if the cache hasn't been invalidated,
// since that can trigger a new write during shutdown, and we don't want to
// cause shutdown hangs.
if (!mCacheInvalidated) {
mal.Wait(10000);
}
- auto result = WriteCache();
+ auto result = URLPreloader::GetSingleton().WriteCache();
+ Unused << NS_WARN_IF(result.isErr());
+
+ result = WriteCache();
Unused << NS_WARN_IF(result.isErr());
result = mChildCache->WriteCache();
Unused << NS_WARN_IF(result.isErr());
mSaveComplete = true;
NS_ReleaseOnMainThreadSystemGroup("ScriptPreloader::mSaveThread",
mSaveThread.forget());
--- a/js/xpconnect/loader/URLPreloader.cpp
+++ b/js/xpconnect/loader/URLPreloader.cpp
@@ -197,16 +197,30 @@ URLPreloader::FindCacheFile()
return Move(cacheFile);
}
Result<Ok, nsresult>
URLPreloader::WriteCache()
{
MOZ_ASSERT(!NS_IsMainThread());
+ // The script preloader might call us a second time, if it has to re-write
+ // its cache after a cache flush. We don't care about cache flushes, since
+ // our cache doesn't store any file data, only paths. And we currently clear
+ // our cached file list after the first write, which means that a second
+ // write would (aside from breaking the invariant that we never touch
+ // mCachedURLs off-main-thread after the first write, and trigger a data
+ // race) mean we get no pre-loading on the next startup.
+ if (mCacheWritten) {
+ return Ok();
+ }
+ mCacheWritten = true;
+
+ LOG(Debug, "Writing cache...");
+
nsCOMPtr<nsIFile> cacheFile;
MOZ_TRY_VAR(cacheFile, GetCacheFile(NS_LITERAL_STRING("-new.bin")));
bool exists;
MOZ_TRY(cacheFile->Exists(&exists));
if (exists) {
MOZ_TRY(cacheFile->Remove(false));
}
@@ -251,16 +265,18 @@ void
URLPreloader::Cleanup()
{
mCachedURLs.Clear();
}
Result<Ok, nsresult>
URLPreloader::ReadCache(LinkedList<URLEntry>& pendingURLs)
{
+ LOG(Debug, "Reading cache...");
+
nsCOMPtr<nsIFile> cacheFile;
MOZ_TRY_VAR(cacheFile, FindCacheFile());
AutoMemMap cache;
MOZ_TRY(cache.init(cacheFile));
auto size = cache.size();
@@ -296,16 +312,18 @@ URLPreloader::ReadCache(LinkedList<URLEn
Range<uint8_t> header(data, data + headerSize);
data += headerSize;
InputBuffer buf(header);
while (!buf.finished()) {
CacheKey key(buf);
+ LOG(Debug, "Cached file: %s %s", key.TypeString(), key.mPath.get());
+
auto entry = mCachedURLs.LookupOrAdd(key, key);
entry->mResultCode = NS_ERROR_NOT_INITIALIZED;
pendingURLs.insertBack(entry);
}
if (buf.error()) {
return Err(NS_ERROR_UNEXPECTED);
@@ -380,16 +398,18 @@ URLPreloader::BackgroundReadFiles()
// If there is any other error code, the entry has already failed at
// this point, so don't bother trying to read it again.
if (entry->mResultCode != NS_ERROR_NOT_INITIALIZED) {
continue;
}
nsresult rv = NS_OK;
+ LOG(Debug, "Background reading %s file %s", entry->TypeString(), entry->mPath.get());
+
if (entry->mType == entry->TypeFile) {
auto result = entry->Read();
if (result.isErr()) {
rv = result.unwrapErr();
}
} else {
auto& cursor = cursors[i++];
--- a/js/xpconnect/loader/URLPreloader.h
+++ b/js/xpconnect/loader/URLPreloader.h
@@ -298,16 +298,19 @@ private:
using HashType = nsClassHashtable<nsGenericHashKey<CacheKey>, URLEntry>;
size_t ShallowSizeOfIncludingThis(mozilla::MallocSizeOf mallocSizeOf);
bool mStartupFinished = false;
bool mReaderInitialized = false;
+ // Only to be accessed from the cache write thread.
+ bool mCacheWritten = false;
+
// The prefix URLs for files in the GRE and App omni jar archives.
nsCString mGREPrefix;
nsCString mAppPrefix;
nsCOMPtr<nsIResProtocolHandler> mResProto;
nsCOMPtr<nsIChromeRegistry> mChromeReg;
nsCOMPtr<nsIFile> mProfD;