mozilla
/
gecko-dev
зеркало из https://github.com/mozilla/gecko-dev.git
1
0
Форкнуть 0
Код Задачи Пакеты Проекты Релизы Вики Активность

Bug 1666214 - Move shutdown timeout handling to QuotaManager. r=dom-workers-and-storage-reviewers,janv 

Instead of having a separate timer for each quota client, we can use a single
timer in the QuotaManager for handling timeouts of all quota clients. The
original shutdown timeout of the quota manager itself can now be removed,
as AbortOperations is already called through InitiateShutdownWorkThreads on
each quota client.

This also moves MaybeRecordShutdownStep to the QuotaManager, which allows
easier access through the singleton instance returned by QuotaManager::GetRef.

Differential Revision: https://phabricator.services.mozilla.com/D98073
This commit is contained in:
Simon Giesecke 2020-12-09 13:03:36 +00:00
Родитель 3ee5c0bd89
Коммит 450b013593
7 изменённых файлов: 144 добавлений и 208 удалений
Показать статистику Скачать Patch файл Скачать Diff файл Показать все файлы Свернуть все файлы

4
dom/cache/Manager.cpp поставляемый
Просмотреть файл

@ -8,7 +8,6 @@
#include "mozilla/AutoRestore.h"
#include "mozilla/Mutex.h"
#include "mozilla/SpinEventLoopUntil.h"
#include "mozilla/StaticMutex.h"
#include "mozilla/StaticPtr.h"
#include "mozilla/Unused.h"
@ -272,7 +271,8 @@ class Manager::Factory {
MOZ_ALWAYS_TRUE(sFactory->mManagerList.RemoveElement(&aManager));
CacheQuotaClient::Get()->MaybeRecordShutdownStep("Manager removed"_ns);
quota::QuotaManager::GetRef().MaybeRecordShutdownStep(
quota::Client::DOMCACHE, "Manager removed"_ns);
// clean up the factory singleton if there are no more managers
MaybeDestroyInstance();

23
dom/indexedDB/ActorsParent.cpp
Просмотреть файл

@ -5108,8 +5108,8 @@ class QuotaClient final : public mozilla::dom::quota::Client {
mCurrentMaintenance = nullptr;
QuotaClient::GetInstance()->MaybeRecordShutdownStep(
"Maintenance finished"_ns);
QuotaManager::GetRef().MaybeRecordShutdownStep(quota::Client::IDB,
"Maintenance finished"_ns);
ProcessMaintenanceQueue();
}
@ -10094,16 +10094,16 @@ void Database::CleanupMetadata() {
MOZ_ALWAYS_TRUE(gLiveDatabaseHashtable->Get(Id(), &info));
MOZ_ALWAYS_TRUE(info->mLiveDatabases.RemoveElement(this));
QuotaClient::GetInstance()->MaybeRecordShutdownStep(
"Live database entry removed"_ns);
QuotaManager::GetRef().MaybeRecordShutdownStep(
quota::Client::IDB, "Live database entry removed"_ns);
if (info->mLiveDatabases.IsEmpty()) {
MOZ_ASSERT(!info->mWaitingFactoryOp ||
!info->mWaitingFactoryOp->HasBlockedDatabases());
gLiveDatabaseHashtable->Remove(Id());
QuotaClient::GetInstance()->MaybeRecordShutdownStep(
"gLiveDatabaseHashtable entry removed"_ns);
QuotaManager::GetRef().MaybeRecordShutdownStep(
quota::Client::IDB, "gLiveDatabaseHashtable entry removed"_ns);
}
// Match the IncreaseBusyCount in OpenDatabaseOp::EnsureDatabaseActor().
@ -15789,11 +15789,12 @@ void FactoryOp::CleanupMetadata() {
MOZ_ASSERT(gFactoryOps);
gFactoryOps->RemoveElement(this);
if (auto* const quotaClient = QuotaClient::GetInstance()) {
quotaClient->MaybeRecordShutdownStep(
"An element was removed from gFactoryOps"_ns);
} else {
NS_WARNING("Cannot record shutdown step because QuotaClient is nullptr");
// We might get here even after QuotaManagerOpen failed, so we need to check
// if we have a quota manager. If we don't, we obviously are not in quota
// manager shutdown.
if (auto* const quotaManager = QuotaManager::Get()) {
quotaManager->MaybeRecordShutdownStep(
quota::Client::IDB, "An element was removed from gFactoryOps"_ns);
}
// Match the IncreaseBusyCount in AllocPBackgroundIDBFactoryRequestParent().

65
dom/localstorage/ActorsParent.cpp
Просмотреть файл

@ -46,7 +46,6 @@
#include "mozilla/ResultExtensions.h"
#include "mozilla/ScopeExit.h"
#include "mozilla/Services.h"
#include "mozilla/SpinEventLoopUntil.h"
#include "mozilla/StaticPrefs_dom.h"
#include "mozilla/StaticPtr.h"
#include "mozilla/StoragePrincipalHelper.h"
@ -4838,14 +4837,8 @@ void Datastore::NoteFinishedPrepareDatastoreOp(
mPrepareDatastoreOps.RemoveEntry(aPrepareDatastoreOp);
// XXX Can we store a strong reference to the quota client ourselves to avoid
// going through mConnection?
if (mConnection) {
mConnection->GetQuotaClient()->MaybeRecordShutdownStep(
"PrepareDatastoreOp finished"_ns);
} else {
NS_WARNING("Cannot record shutdown step, mConnection is nullptr");
}
QuotaManager::GetRef().MaybeRecordShutdownStep(
quota::Client::LS, "PrepareDatastoreOp finished"_ns);
MaybeClose();
}
@ -4867,14 +4860,8 @@ void Datastore::NoteFinishedPrivateDatastore() {
mHasLivePrivateDatastore = false;
// XXX Can we store a strong reference to the quota client ourselves to avoid
// going through mConnection?
if (mConnection) {
mConnection->GetQuotaClient()->MaybeRecordShutdownStep(
"PrivateDatastore finished"_ns);
} else {
NS_WARNING("Cannot record shutdown step, mConnection is nullptr");
}
QuotaManager::GetRef().MaybeRecordShutdownStep(
quota::Client::LS, "PrivateDatastore finished"_ns);
MaybeClose();
}
@ -4900,14 +4887,8 @@ void Datastore::NoteFinishedPreparedDatastore(
mPreparedDatastores.RemoveEntry(aPreparedDatastore);
// XXX Can we store a strong reference to the quota client ourselves to avoid
// going through mConnection?
if (mConnection) {
mConnection->GetQuotaClient()->MaybeRecordShutdownStep(
"PreparedDatastore finished"_ns);
} else {
NS_WARNING("Cannot record shutdown step, mConnection is nullptr");
}
QuotaManager::GetRef().MaybeRecordShutdownStep(
quota::Client::LS, "PreparedDatastore finished"_ns);
MaybeClose();
}
@ -4932,14 +4913,8 @@ void Datastore::NoteFinishedDatabase(Database* aDatabase) {
mDatabases.RemoveEntry(aDatabase);
// XXX Can we store a strong reference to the quota client ourselves to avoid
// going through mConnection?
if (mConnection) {
mConnection->GetQuotaClient()->MaybeRecordShutdownStep(
"Database finished"_ns);
} else {
NS_WARNING("Cannot record shutdown step, mConnection is nullptr");
}
QuotaManager::GetRef().MaybeRecordShutdownStep(quota::Client::LS,
"Database finished"_ns);
MaybeClose();
}
@ -5600,14 +5575,8 @@ void Datastore::CleanupMetadata() {
MOZ_ASSERT(gDatastores->Get(mGroupAndOrigin.mOrigin));
gDatastores->Remove(mGroupAndOrigin.mOrigin);
// XXX Can we store a strong reference to the quota client ourselves to avoid
// going through mConnection?
if (mConnection) {
mConnection->GetQuotaClient()->MaybeRecordShutdownStep(
"Datastore removed"_ns);
} else {
NS_WARNING("Cannot record shutdown step, mConnection is nullptr");
}
QuotaManager::GetRef().MaybeRecordShutdownStep(quota::Client::LS,
"Datastore removed"_ns);
if (!gDatastores->Count()) {
gDatastores = nullptr;
@ -5806,8 +5775,8 @@ void Database::AllowToClose() {
MOZ_ASSERT(gLiveDatabases);
gLiveDatabases->RemoveElement(this);
// XXX Record removal from gLiveDatabase here as well, once we have an easy
// way to record a shutdown step here.
QuotaManager::GetRef().MaybeRecordShutdownStep(quota::Client::LS,
"Live database removed"_ns);
if (gLiveDatabases->IsEmpty()) {
gLiveDatabases = nullptr;
@ -8040,14 +8009,8 @@ void PrepareDatastoreOp::CleanupMetadata() {
MOZ_ASSERT(gPrepareDatastoreOps);
gPrepareDatastoreOps->RemoveElement(this);
// XXX Can we store a strong reference to the quota client ourselves to avoid
// going through mConnection?
if (mConnection) {
mConnection->GetQuotaClient()->MaybeRecordShutdownStep(
"PrepareDatastoreOp completed"_ns);
} else {
NS_WARNING("Cannot record shutdown step, mConnection is nullptr");
}
QuotaManager::GetRef().MaybeRecordShutdownStep(
quota::Client::LS, "PrepareDatastoreOp completed"_ns);
if (gPrepareDatastoreOps->IsEmpty()) {
gPrepareDatastoreOps = nullptr;

123
dom/quota/ActorsParent.cpp
Просмотреть файл

@ -179,9 +179,23 @@
// after the last event it processes.
#define DEFAULT_THREAD_TIMEOUT_MS 30000
// The amount of time, in milliseconds, that we will wait for active storage
// transactions on shutdown before aborting them.
#define DEFAULT_SHUTDOWN_TIMER_MS 30000
/**
* If shutdown takes this long, kill actors of a quota client, to avoid reaching
* the crash timeout.
*/
#define SHUTDOWN_FORCE_KILL_TIMEOUT_MS 5000
/**
* Automatically crash the browser if shutdown of a quota client takes this
* long. We've chosen a value that is long enough that it is unlikely for the
* problem to be falsely triggered by slow system I/O. We've also chosen a
* value long enough so that automated tests should time out and fail if
* shutdown of a quota client takes too long. Also, this value is long enough
* so that testers can notice the timeout; we want to know about the timeouts,
* not hide them. On the other hand this value is less than 60 seconds which is
* used by nsTerminator to crash a hung main process.
*/
#define SHUTDOWN_FORCE_CRASH_TIMEOUT_MS 45000
// profile-before-change, when we need to shut down quota manager
#define PROFILE_BEFORE_CHANGE_QM_OBSERVER_ID "profile-before-change-qm"
@ -3657,6 +3671,13 @@ QuotaManager* QuotaManager::Get() {
return gInstance;
}
// static
QuotaManager& QuotaManager::GetRef() {
MOZ_ASSERT(gInstance);
return *gInstance;
}
// static
bool QuotaManager::IsShuttingDown() { return gShutdown; }
@ -4067,6 +4088,37 @@ nsresult QuotaManager::Init() {
return NS_OK;
}
void QuotaManager::MaybeRecordShutdownStep(const Client::Type aClientType,
const nsACString& aStepDescription) {
AssertIsOnBackgroundThread();
if (!mShutdownStartedAt) {
// We are not shutting down yet, we intentionally ignore this here to avoid
// that every caller has to make a distinction for shutdown vs. non-shutdown
// situations.
return;
}
const TimeDuration elapsedSinceShutdownStart =
TimeStamp::NowLoRes() - *mShutdownStartedAt;
const auto stepString =
nsPrintfCString("%fs: %s", elapsedSinceShutdownStart.ToSeconds(),
nsPromiseFlatCString(aStepDescription).get());
mShutdownSteps[aClientType].Append(stepString + "\n"_ns);
#ifdef DEBUG
// XXX Probably this isn't the mechanism that should be used here.
NS_DebugBreak(
NS_DEBUG_WARNING,
nsAutoCString(Client::TypeToText(aClientType) + " shutdown step"_ns)
.get(),
stepString.get(), __FILE__, __LINE__);
#endif
}
void QuotaManager::Shutdown() {
AssertIsOnOwningThread();
@ -4078,10 +4130,7 @@ void QuotaManager::Shutdown() {
StopIdleMaintenance();
// Kick off the shutdown timer.
MOZ_ALWAYS_SUCCEEDS(mShutdownTimer->InitWithNamedFuncCallback(
&ShutdownTimerCallback, this, DEFAULT_SHUTDOWN_TIMER_MS,
nsITimer::TYPE_ONE_SHOT, "QuotaManager::ShutdownTimerCallback"));
mShutdownStartedAt.init(TimeStamp::NowLoRes());
const auto& allClientTypes = AllClientTypes();
@ -4091,9 +4140,51 @@ void QuotaManager::Shutdown() {
}
needsToWait |= static_cast<bool>(gNormalOriginOps);
// If any client cannot shutdown immediately, spin the event loop while we
// If any clients cannot shutdown immediately, spin the event loop while we
// wait on all the threads to close. Our timer may fire during that loop.
if (needsToWait) {
MOZ_ALWAYS_SUCCEEDS(mShutdownTimer->InitWithNamedFuncCallback(
[](nsITimer* aTimer, void* aClosure) {
auto* const quotaManager = static_cast<QuotaManager*>(aClosure);
for (Client::Type type : quotaManager->AllClientTypes()) {
quotaManager->mClients[type]->ForceKillActors();
}
MOZ_ALWAYS_SUCCEEDS(aTimer->InitWithNamedFuncCallback(
[](nsITimer* aTimer, void* aClosure) {
auto* const quotaManager = static_cast<QuotaManager*>(aClosure);
nsCString annotation;
for (Client::Type type : quotaManager->AllClientTypes()) {
auto& quotaClient = *quotaManager->mClients[type];
if (!quotaClient.IsShutdownCompleted()) {
annotation.Append(nsPrintfCString(
"%s: %s\nIntermediate steps:\n%s\n\n",
Client::TypeToText(type).get(),
quotaClient.GetShutdownStatus().get(),
quotaManager->mShutdownSteps[type].get()));
}
}
// We expect that at least one quota client didn't complete its
// shutdown.
MOZ_DIAGNOSTIC_ASSERT(!annotation.IsEmpty());
CrashReporter::AnnotateCrashReport(
CrashReporter::Annotation::QuotaManagerShutdownTimeout,
annotation.get());
MOZ_CRASH("Quota manager shutdown timed out");
},
aClosure, SHUTDOWN_FORCE_CRASH_TIMEOUT_MS,
nsITimer::TYPE_ONE_SHOT, "quota::QuotaManager::ForceCrashTimer"));
},
this, SHUTDOWN_FORCE_KILL_TIMEOUT_MS, nsITimer::TYPE_ONE_SHOT,
"quota::QuotaManager::ForceKillTimer"));
MOZ_ALWAYS_TRUE(SpinEventLoopUntil([this, &allClientTypes] {
return !gNormalOriginOps &&
std::all_of(allClientTypes.cbegin(), allClientTypes.cend(),
@ -7622,22 +7713,6 @@ void QuotaManager::FinalizeOriginEviction(
}
}
void QuotaManager::ShutdownTimerCallback(nsITimer* aTimer, void* aClosure) {
AssertIsOnBackgroundThread();
auto quotaManager = static_cast<QuotaManager*>(aClosure);
MOZ_ASSERT(quotaManager);
NS_WARNING(
"Some storage operations are taking longer than expected "
"during shutdown and will be aborted!");
// Abort all operations.
for (RefPtr<Client>& client : quotaManager->mClients) {
client->AbortAllOperations();
}
}
auto QuotaManager::GetDirectoryLockTable(PersistenceType aPersistenceType)
-> DirectoryLockTable& {
switch (aPersistenceType) {

110
dom/quota/Client.cpp
Просмотреть файл

@ -9,7 +9,7 @@
// Global includes
#include "BackgroundParent.h"
#include "mozilla/Assertions.h"
#include "mozilla/SpinEventLoopUntil.h"
#include "mozilla/dom/quota/QuotaManager.h"
namespace mozilla::dom::quota {
@ -22,27 +22,6 @@ const char kDOMCachePrefix = 'C';
const char kSDBPrefix = 'S';
const char kLSPrefix = 'L';
/**
* If shutdown takes this long, kill actors of a quota client, to avoid reaching
* the crash timeout.
*/
#define SHUTDOWN_FORCE_KILL_TIMEOUT_MS 5000
/**
* Automatically crash the browser if shutdown of a quota client takes this
* long. We've chosen a value that is longer than the value for QuotaManager
* shutdown timer which is currently set to 30 seconds, so that the QuotaManager
* abort may still make the quota client's shutdown complete in time. We've
* also chosen a value that is long enough that it is unlikely for the problem
* to be falsely triggered by slow system I/O. We've also chosen a value long
* enough so that automated tests should time out and fail if shutdown of a
* quota client takes too long. Also, this value is long enough so that testers
* can notice the timeout; we want to know about the timeouts, not hide them. On
* the other hand this value is less than 60 seconds which is used by
* nsTerminator to crash a hung main process.
*/
#define SHUTDOWN_FORCE_CRASH_TIMEOUT_MS 45000
template <Client::Type type>
struct ClientTypeTraits;
@ -251,99 +230,18 @@ bool Client::TypeFromPrefix(char aPrefix, Type& aType, const fallible_t&) {
return true;
}
void Client::MaybeRecordShutdownStep(const nsACString& aStepDescription) {
AssertIsOnBackgroundThread();
if (!mShutdownStartedAt) {
// We are not shutting down yet, we intentionally ignore this here to avoid
// that every caller has to make a distinction for shutdown vs. non-shutdown
// situations.
return;
}
const TimeDuration elapsedSinceShutdownStart =
TimeStamp::NowLoRes() - *mShutdownStartedAt;
const auto stepString =
nsPrintfCString("%fs: %s", elapsedSinceShutdownStart.ToSeconds(),
nsPromiseFlatCString(aStepDescription).get());
mShutdownSteps.Append(stepString + "\n"_ns);
#ifdef DEBUG
// XXX Probably this isn't the mechanism that should be used here.
NS_DebugBreak(
NS_DEBUG_WARNING,
nsAutoCString(TypeToText(GetType()) + " shutdown step"_ns).get(),
stepString.get(), __FILE__, __LINE__);
#endif
}
bool Client::InitiateShutdownWorkThreads() {
AssertIsOnBackgroundThread();
MOZ_ASSERT(!mShutdownTimer);
MOZ_ASSERT(mShutdownSteps.IsEmpty());
mShutdownStartedAt.init(TimeStamp::NowLoRes());
MaybeRecordShutdownStep("starting"_ns);
QuotaManager::GetRef().MaybeRecordShutdownStep(GetType(), "starting"_ns);
InitiateShutdown();
if (!IsShutdownCompleted()) {
mShutdownTimer = NS_NewTimer();
MOZ_ALWAYS_SUCCEEDS(mShutdownTimer->InitWithNamedFuncCallback(
[](nsITimer* aTimer, void* aClosure) {
auto* const quotaClient = static_cast<Client*>(aClosure);
quotaClient->ForceKillActors();
MOZ_ALWAYS_SUCCEEDS(aTimer->InitWithNamedFuncCallback(
[](nsITimer* aTimer, void* aClosure) {
auto* const quotaClient = static_cast<Client*>(aClosure);
if (quotaClient->IsShutdownCompleted()) {
// Apparently, the shutdown did complete between the timer
// fired and we processed the event. So there's no reason to
// crash, just do nothing.
return;
}
const auto type = TypeToText(quotaClient->GetType());
CrashReporter::AnnotateCrashReport(
CrashReporter::Annotation::QuotaManagerShutdownTimeout,
nsPrintfCString("%s: %s\nIntermediate steps:\n%s",
type.get(),
quotaClient->GetShutdownStatus().get(),
quotaClient->mShutdownSteps.get()));
MOZ_CRASH_UNSAFE_PRINTF("%s shutdown timed out", type.get());
},
aClosure, SHUTDOWN_FORCE_CRASH_TIMEOUT_MS,
nsITimer::TYPE_ONE_SHOT,
"quota::Client::ShutdownWorkThreads::ForceCrashTimer"));
},
this, SHUTDOWN_FORCE_KILL_TIMEOUT_MS, nsITimer::TYPE_ONE_SHOT,
"quota::Client::ShutdownWorkThreads::ForceKillTimer"));
return true;
}
return false;
return IsShutdownCompleted();
}
void Client::FinalizeShutdownWorkThreads() {
MOZ_ASSERT(mShutdownStartedAt);
if (mShutdownTimer) {
MOZ_ALWAYS_SUCCEEDS(mShutdownTimer->Cancel());
}
MaybeRecordShutdownStep("completed"_ns);
QuotaManager::GetRef().MaybeRecordShutdownStep(GetType(), "completed"_ns);
FinalizeShutdown();
}

16
dom/quota/Client.h
Просмотреть файл

@ -10,17 +10,16 @@
#include <cstdint>
#include "ErrorList.h"
#include "mozilla/Atomics.h"
#include "mozilla/InitializedOnce.h"
#include "mozilla/Result.h"
#include "mozilla/dom/LocalStorageCommon.h"
#include "mozilla/dom/ipc/IdType.h"
#include "mozilla/dom/quota/PersistenceType.h"
#include "mozilla/dom/quota/QuotaCommon.h"
#include "mozilla/dom/quota/QuotaInfo.h"
#include "mozilla/fallible.h"
#include "nsHashKeys.h"
#include "nsISupports.h"
#include "nsStringFwd.h"
#include "nsTHashtable.h"
class nsIFile;
@ -42,6 +41,7 @@ BEGIN_QUOTA_NAMESPACE
class OriginScope;
class QuotaManager;
class UsageInfo;
struct GroupAndOrigin;
// An abstract interface for quota manager clients.
// Each storage API must provide an implementation of this interface in order
@ -172,22 +172,14 @@ class Client {
bool InitiateShutdownWorkThreads();
void FinalizeShutdownWorkThreads();
virtual nsCString GetShutdownStatus() const = 0;
virtual bool IsShutdownCompleted() const = 0;
void MaybeRecordShutdownStep(const nsACString& aStepDescription);
virtual void ForceKillActors() = 0;
private:
virtual void InitiateShutdown() = 0;
virtual nsCString GetShutdownStatus() const = 0;
virtual void ForceKillActors() = 0;
virtual void FinalizeShutdown() = 0;
// A timer that gets activated at shutdown to ensure we close all storages.
nsCOMPtr<nsITimer> mShutdownTimer;
nsCString mShutdownSteps;
LazyInitializedOnce<const TimeStamp> mShutdownStartedAt;
protected:
virtual ~Client() = default;
};

11
dom/quota/QuotaManager.h
Просмотреть файл

@ -13,6 +13,7 @@
#include "ErrorList.h"
#include "mozilla/AlreadyAddRefed.h"
#include "mozilla/Assertions.h"
#include "mozilla/InitializedOnce.h"
#include "mozilla/Mutex.h"
#include "mozilla/RefPtr.h"
#include "mozilla/Result.h"
@ -154,6 +155,8 @@ class QuotaManager final : public BackgroundThreadObject {
// Returns a non-owning reference.
static QuotaManager* Get();
static QuotaManager& GetRef();
// Returns true if we've begun the shutdown process.
static bool IsShuttingDown();
@ -422,6 +425,9 @@ class QuotaManager final : public BackgroundThreadObject {
void NotifyStoragePressure(uint64_t aUsage);
void MaybeRecordShutdownStep(Client::Type aClientType,
const nsACString& aStepDescription);
static void GetStorageId(PersistenceType aPersistenceType,
const nsACString& aOrigin, Client::Type aClientType,
nsACString& aDatabaseId);
@ -567,8 +573,6 @@ class QuotaManager final : public BackgroundThreadObject {
int64_t GenerateDirectoryLockId();
static void ShutdownTimerCallback(nsITimer* aTimer, void* aClosure);
// Thread on which IO is performed.
nsCOMPtr<nsIThread> mIOThread;
@ -577,6 +581,9 @@ class QuotaManager final : public BackgroundThreadObject {
// A timer that gets activated at shutdown to ensure we close all storages.
nsCOMPtr<nsITimer> mShutdownTimer;
EnumeratedArray<Client::Type, Client::TYPE_MAX, nsCString> mShutdownSteps;
LazyInitializedOnce<const TimeStamp> mShutdownStartedAt;
mozilla::Mutex mQuotaMutex;
nsClassHashtable<nsCStringHashKey, GroupInfoPair> mGroupInfoPairs;