Backed out changeset 547a95c0bb90 (bug 1666214) for causing talos failures.

CLOSED TREE
This commit is contained in:
Mihai Alexandru Michis 2020-12-09 22:08:57 +02:00
Родитель 6cc508ffcb
Коммит 783558e615
7 изменённых файлов: 209 добавлений и 145 удалений

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

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

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

@ -5126,8 +5126,8 @@ class QuotaClient final : public mozilla::dom::quota::Client {
mCurrentMaintenance = nullptr;
QuotaManager::GetRef().MaybeRecordShutdownStep(quota::Client::IDB,
"Maintenance finished"_ns);
QuotaClient::GetInstance()->MaybeRecordShutdownStep(
"Maintenance finished"_ns);
ProcessMaintenanceQueue();
}
@ -10124,16 +10124,16 @@ void Database::CleanupMetadata() {
MOZ_ALWAYS_TRUE(gLiveDatabaseHashtable->Get(Id(), &info));
MOZ_ALWAYS_TRUE(info->mLiveDatabases.RemoveElement(this));
QuotaManager::GetRef().MaybeRecordShutdownStep(
quota::Client::IDB, "Live database entry removed"_ns);
QuotaClient::GetInstance()->MaybeRecordShutdownStep(
"Live database entry removed"_ns);
if (info->mLiveDatabases.IsEmpty()) {
MOZ_ASSERT(!info->mWaitingFactoryOp ||
!info->mWaitingFactoryOp->HasBlockedDatabases());
gLiveDatabaseHashtable->Remove(Id());
QuotaManager::GetRef().MaybeRecordShutdownStep(
quota::Client::IDB, "gLiveDatabaseHashtable entry removed"_ns);
QuotaClient::GetInstance()->MaybeRecordShutdownStep(
"gLiveDatabaseHashtable entry removed"_ns);
}
// Match the IncreaseBusyCount in OpenDatabaseOp::EnsureDatabaseActor().
@ -15821,12 +15821,11 @@ void FactoryOp::CleanupMetadata() {
MOZ_ASSERT(gFactoryOps);
gFactoryOps->RemoveElement(this);
// 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);
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");
}
// Match the IncreaseBusyCount in AllocPBackgroundIDBFactoryRequestParent().

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

@ -46,6 +46,7 @@
#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"
@ -4837,8 +4838,14 @@ void Datastore::NoteFinishedPrepareDatastoreOp(
mPrepareDatastoreOps.RemoveEntry(aPrepareDatastoreOp);
QuotaManager::GetRef().MaybeRecordShutdownStep(
quota::Client::LS, "PrepareDatastoreOp finished"_ns);
// 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");
}
MaybeClose();
}
@ -4860,8 +4867,14 @@ void Datastore::NoteFinishedPrivateDatastore() {
mHasLivePrivateDatastore = false;
QuotaManager::GetRef().MaybeRecordShutdownStep(
quota::Client::LS, "PrivateDatastore finished"_ns);
// 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");
}
MaybeClose();
}
@ -4887,8 +4900,14 @@ void Datastore::NoteFinishedPreparedDatastore(
mPreparedDatastores.RemoveEntry(aPreparedDatastore);
QuotaManager::GetRef().MaybeRecordShutdownStep(
quota::Client::LS, "PreparedDatastore finished"_ns);
// 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");
}
MaybeClose();
}
@ -4913,8 +4932,14 @@ void Datastore::NoteFinishedDatabase(Database* aDatabase) {
mDatabases.RemoveEntry(aDatabase);
QuotaManager::GetRef().MaybeRecordShutdownStep(quota::Client::LS,
"Database finished"_ns);
// 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");
}
MaybeClose();
}
@ -5575,8 +5600,14 @@ void Datastore::CleanupMetadata() {
MOZ_ASSERT(gDatastores->Get(mGroupAndOrigin.mOrigin));
gDatastores->Remove(mGroupAndOrigin.mOrigin);
QuotaManager::GetRef().MaybeRecordShutdownStep(quota::Client::LS,
"Datastore removed"_ns);
// 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");
}
if (!gDatastores->Count()) {
gDatastores = nullptr;
@ -5775,8 +5806,8 @@ void Database::AllowToClose() {
MOZ_ASSERT(gLiveDatabases);
gLiveDatabases->RemoveElement(this);
QuotaManager::GetRef().MaybeRecordShutdownStep(quota::Client::LS,
"Live database removed"_ns);
// XXX Record removal from gLiveDatabase here as well, once we have an easy
// way to record a shutdown step here.
if (gLiveDatabases->IsEmpty()) {
gLiveDatabases = nullptr;
@ -8009,8 +8040,14 @@ void PrepareDatastoreOp::CleanupMetadata() {
MOZ_ASSERT(gPrepareDatastoreOps);
gPrepareDatastoreOps->RemoveElement(this);
QuotaManager::GetRef().MaybeRecordShutdownStep(
quota::Client::LS, "PrepareDatastoreOp completed"_ns);
// 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");
}
if (gPrepareDatastoreOps->IsEmpty()) {
gPrepareDatastoreOps = nullptr;

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

@ -179,23 +179,9 @@
// after the last event it processes.
#define DEFAULT_THREAD_TIMEOUT_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
// 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
// profile-before-change, when we need to shut down quota manager
#define PROFILE_BEFORE_CHANGE_QM_OBSERVER_ID "profile-before-change-qm"
@ -3671,13 +3657,6 @@ QuotaManager* QuotaManager::Get() {
return gInstance;
}
// static
QuotaManager& QuotaManager::GetRef() {
MOZ_ASSERT(gInstance);
return *gInstance;
}
// static
bool QuotaManager::IsShuttingDown() { return gShutdown; }
@ -4088,37 +4067,6 @@ 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();
@ -4130,7 +4078,10 @@ void QuotaManager::Shutdown() {
StopIdleMaintenance();
mShutdownStartedAt.init(TimeStamp::NowLoRes());
// Kick off the shutdown timer.
MOZ_ALWAYS_SUCCEEDS(mShutdownTimer->InitWithNamedFuncCallback(
&ShutdownTimerCallback, this, DEFAULT_SHUTDOWN_TIMER_MS,
nsITimer::TYPE_ONE_SHOT, "QuotaManager::ShutdownTimerCallback"));
const auto& allClientTypes = AllClientTypes();
@ -4140,51 +4091,9 @@ void QuotaManager::Shutdown() {
}
needsToWait |= static_cast<bool>(gNormalOriginOps);
// If any clients cannot shutdown immediately, spin the event loop while we
// If any client 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(),
@ -7713,6 +7622,22 @@ 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) {

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

@ -9,7 +9,7 @@
// Global includes
#include "BackgroundParent.h"
#include "mozilla/Assertions.h"
#include "mozilla/dom/quota/QuotaManager.h"
#include "mozilla/SpinEventLoopUntil.h"
namespace mozilla::dom::quota {
@ -22,6 +22,27 @@ 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;
@ -230,18 +251,99 @@ bool Client::TypeFromPrefix(char aPrefix, Type& aType, const fallible_t&) {
return true;
}
bool Client::InitiateShutdownWorkThreads() {
void Client::MaybeRecordShutdownStep(const nsACString& aStepDescription) {
AssertIsOnBackgroundThread();
QuotaManager::GetRef().MaybeRecordShutdownStep(GetType(), "starting"_ns);
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);
InitiateShutdown();
return IsShutdownCompleted();
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;
}
void Client::FinalizeShutdownWorkThreads() {
QuotaManager::GetRef().MaybeRecordShutdownStep(GetType(), "completed"_ns);
MOZ_ASSERT(mShutdownStartedAt);
if (mShutdownTimer) {
MOZ_ALWAYS_SUCCEEDS(mShutdownTimer->Cancel());
}
MaybeRecordShutdownStep("completed"_ns);
FinalizeShutdown();
}

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

@ -10,16 +10,17 @@
#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;
@ -41,7 +42,6 @@ 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,14 +172,22 @@ class Client {
bool InitiateShutdownWorkThreads();
void FinalizeShutdownWorkThreads();
virtual nsCString GetShutdownStatus() const = 0;
virtual bool IsShutdownCompleted() const = 0;
virtual void ForceKillActors() = 0;
void MaybeRecordShutdownStep(const nsACString& aStepDescription);
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;
};

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

@ -13,7 +13,6 @@
#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"
@ -155,8 +154,6 @@ 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();
@ -425,9 +422,6 @@ 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);
@ -573,6 +567,8 @@ class QuotaManager final : public BackgroundThreadObject {
int64_t GenerateDirectoryLockId();
static void ShutdownTimerCallback(nsITimer* aTimer, void* aClosure);
// Thread on which IO is performed.
nsCOMPtr<nsIThread> mIOThread;
@ -581,9 +577,6 @@ 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;