Bug 1656261 - Ensure we clean up after WriteToDisk r=froydnj

Given all of the MOZ_TRYs in WriteToDisk, it's entirely possible that
we occasionally exit it before writing everything out. WriteToDisk
additionally adds guards to try to ensure that we do not call it twice,
and that we do not try to read things out of the file via the memmapped
buffer after it has written to it. The latter should not generally be
a problem, since we write to a temp file and move that over at the end.
However, writing twice could be a problem as we modify our values'
mOffsets to keep track of their offsets within the resulting file.

Accordingly, this change simply moves the cleanup and guards into an
RAII helper to ensure they survive all of the early returns.

Differential Revision: https://phabricator.services.mozilla.com/D89253
This commit is contained in:
Doug Thayer 2020-09-09 13:42:27 +00:00
Родитель bfa4d94201
Коммит 2b194eb9d0
1 изменённых файлов: 21 добавлений и 18 удалений

Просмотреть файл

@ -1053,6 +1053,27 @@ Result<Ok, nsresult> StartupCache::WriteToDisk() {
mFile->GetNativeLeafName(leafName);
tmpFile->SetNativeLeafName(leafName + "-tmp"_ns);
auto cleanup = MakeScopeExit([&]() {
mDirty = false;
mWrittenOnce = true;
mCacheData.reset();
// If we're shutting down, we do not care about freeing up memory right now.
if (AppShutdown::IsShuttingDown()) {
return;
}
// We've just written (or attempted to write) our buffers to disk, and
// we're 60 seconds out from startup, so they're unlikely to be needed
// again any time soon; let's clean up what we can.
for (auto iter = mTable.iter(); !iter.done(); iter.next()) {
auto& value = iter.get().value();
if (!value.mFlags.contains(StartupCacheEntryFlags::DoNotFree)) {
value.mData = nullptr;
}
}
});
{
AutoFDClose fd;
MOZ_TRY(tmpFile->OpenNSPRFileDesc(PR_WRONLY | PR_CREATE_FILE | PR_TRUNCATE,
@ -1183,26 +1204,8 @@ Result<Ok, nsresult> StartupCache::WriteToDisk() {
MOZ_TRY(Write(fd, buf.Get(), buf.cursor()));
}
mDirty = false;
mWrittenOnce = true;
mCacheData.reset();
tmpFile->MoveToNative(nullptr, leafName);
// If we're shutting down, we do not care about freeing up memory right now.
if (AppShutdown::IsShuttingDown()) {
return Ok();
}
// We've just written our buffers to disk, and we're 60 seconds out from
// startup, so they're unlikely to be needed again any time soon; let's
// clean up what we can.
for (auto iter = mTable.iter(); !iter.done(); iter.next()) {
auto& value = iter.get().value();
if (!value.mFlags.contains(StartupCacheEntryFlags::DoNotFree)) {
value.mData = nullptr;
}
}
return Ok();
}