Skip to content

Commit

Permalink
Bug 1644896 - Destroy PermissionManager on shutdown, r=timhuang
Browse files Browse the repository at this point in the history 
Differential Revision: https://phabricator.services.mozilla.com/D82211
  • Loading branch information
@bakulf
bakulf committed Jul 13, 2020
1 parent 9be28a6 commit 8a5c2d5
Show file tree
Hide file tree
Showing 2 changed files with 92 additions and 31 deletions.
106 changes: 78 additions & 28 deletions extensions/permissions/PermissionManager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
#include "nsIPrefBranch.h"
#include "nsIPrincipal.h"
#include "nsIURIMutator.h"
#include "nsIWritablePropertyBag2.h"
#include "nsReadLine.h"
#include "nsToolkitCompsCID.h"

Expand Down Expand Up @@ -611,7 +612,7 @@ void PermissionManager::Startup() {
// PermissionManager Implementation

NS_IMPL_ISUPPORTS(PermissionManager, nsIPermissionManager, nsIObserver,
nsISupportsWeakReference)
nsISupportsWeakReference, nsIAsyncShutdownBlocker)

PermissionManager::PermissionManager()
: mMonitor("PermissionManager::mMonitor"),
Expand All @@ -629,12 +630,6 @@ PermissionManager::~PermissionManager() {
}
mPermissionKeyPromiseMap.Clear();

RemoveAllFromMemory();
if (gPermissionManager) {
MOZ_ASSERT(gPermissionManager == this);
gPermissionManager = nullptr;
}

if (mThread) {
mThread->Shutdown();
mThread = nullptr;
Expand All @@ -655,9 +650,7 @@ already_AddRefed<nsIPermissionManager> PermissionManager::GetXPCOMSingleton() {
// See bug 209571.
auto permManager = MakeRefPtr<PermissionManager>();
if (NS_SUCCEEDED(permManager->Init())) {
// Note: This is cleared in the PermissionManager destructor.
gPermissionManager = permManager.get();
ClearOnShutdown(&gPermissionManager);
return permManager.forget();
}

Expand Down Expand Up @@ -692,12 +685,15 @@ nsresult PermissionManager::Init() {
// Stop here; we don't need the DB in the child process. Instead we will be
// sent permissions as we need them by our parent process.
mState = eReady;

// We use ClearOnShutdown on the content process only because on the parent
// process we need to block the shutdown for the final closeDB() call.
ClearOnShutdown(&gPermissionManager);
return NS_OK;
}

nsCOMPtr<nsIObserverService> observerService = services::GetObserverService();
if (observerService) {
observerService->AddObserver(this, "profile-before-change", true);
observerService->AddObserver(this, "profile-do-change", true);
observerService->AddObserver(this, "testonly-reload-permissions-from-disk",
true);
Expand Down Expand Up @@ -1919,8 +1915,6 @@ PermissionManager::RemoveAllSince(int64_t aSince) {

template <class T>
nsresult PermissionManager::RemovePermissionEntries(T aCondition) {
EnsureReadCompleted();

Vector<Tuple<nsCOMPtr<nsIPrincipal>, nsCString, nsCString>, 10> array;
for (auto iter = mPermissionTable.Iter(); !iter.Done(); iter.Next()) {
PermissionHashKey* entry = iter.Get();
Expand Down Expand Up @@ -1995,20 +1989,19 @@ PermissionManager::RemoveByTypeSince(const nsACString& aType,
});
}

void PermissionManager::CloseDB(bool aRebuildOnSuccess) {
void PermissionManager::CloseDB(CloseDBNextOp aNextOp) {
EnsureReadCompleted();

mState = eClosed;

nsCOMPtr<nsIInputStream> defaultsInputStream;
if (aRebuildOnSuccess) {
if (aNextOp == eRebuldOnSuccess) {
defaultsInputStream = GetDefaultsInputStream();
}

RefPtr<PermissionManager> self = this;
mThread->Dispatch(NS_NewRunnableFunction(
"PermissionManager::CloseDB",
[self, aRebuildOnSuccess, defaultsInputStream] {
"PermissionManager::CloseDB", [self, aNextOp, defaultsInputStream] {
auto data = self->mThreadBoundData.Access();
// Null the statements, this will finalize them.
data->mStmtInsert = nullptr;
Expand All @@ -2019,10 +2012,16 @@ void PermissionManager::CloseDB(bool aRebuildOnSuccess) {
MOZ_ASSERT(NS_SUCCEEDED(rv));
data->mDBConn = nullptr;

if (aRebuildOnSuccess) {
if (aNextOp == eRebuldOnSuccess) {
self->TryInitDB(true, defaultsInputStream);
}
}

if (aNextOp == eShutdown) {
NS_DispatchToMainThread(NS_NewRunnableFunction(
"PermissionManager::MaybeCompleteShutdown",
[self] { self->MaybeCompleteShutdown(); }));
}
}));
}

Expand Down Expand Up @@ -2076,7 +2075,7 @@ nsresult PermissionManager::RemoveAllInternal(bool aNotifyObservers) {
if (NS_WARN_IF(NS_FAILED(rv))) {
NS_DispatchToMainThread(NS_NewRunnableFunction(
"PermissionManager::RemoveAllInternal-Failure",
[self] { self->CloseDB(true); }));
[self] { self->CloseDB(eRebuldOnSuccess); }));
}
}));

Expand Down Expand Up @@ -2357,15 +2356,16 @@ NS_IMETHODIMP PermissionManager::Observe(nsISupports* aSubject,
const char16_t* someData) {
ENSURE_NOT_CHILD_PROCESS;

if (!nsCRT::strcmp(aTopic, "profile-before-change")) {
// The profile is about to change,
// or is going away because the application is shutting down.
RemoveIdleDailyMaintenanceJob();
RemoveAllFromMemory();
CloseDB(false);
} else if (!nsCRT::strcmp(aTopic, "profile-do-change")) {
if (!nsCRT::strcmp(aTopic, "profile-do-change")) {
// the profile has already changed; init the db from the new location
InitDB(false);

nsAutoString blockerName;
MOZ_ALWAYS_SUCCEEDS(GetName(blockerName));

Unused << NS_WARN_IF(NS_FAILED(GetShutdownPhase()->AddBlocker(
this, NS_LITERAL_STRING_FROM_CSTRING(__FILE__), __LINE__,
blockerName)));
} else if (!nsCRT::strcmp(aTopic, "testonly-reload-permissions-from-disk")) {
// Testing mechanism to reload all permissions from disk. Because the
// permission manager automatically initializes itself at startup, tests
Expand All @@ -2375,7 +2375,7 @@ NS_IMETHODIMP PermissionManager::Observe(nsISupports* aSubject,
// always being initialized. This is not guarded by a pref because it's not
// dangerous to reload permissions from disk, just bad for performance.
RemoveAllFromMemory();
CloseDB(false);
CloseDB(eNone);
InitDB(false);
} else if (!nsCRT::strcmp(aTopic, OBSERVER_TOPIC_IDLE_DAILY)) {
PerformIdleDailyMaintenance();
Expand Down Expand Up @@ -2406,8 +2406,6 @@ PermissionManager::RemovePermissionsWithAttributes(const nsAString& aPattern) {

nsresult PermissionManager::RemovePermissionsWithAttributes(
OriginAttributesPattern& aPattern) {
EnsureReadCompleted();

Vector<Tuple<nsCOMPtr<nsIPrincipal>, nsCString, nsCString>, 10> permissions;
for (auto iter = mPermissionTable.Iter(); !iter.Done(); iter.Next()) {
PermissionHashKey* entry = iter.Get();
Expand Down Expand Up @@ -3546,4 +3544,56 @@ nsresult PermissionManager::TestPermissionWithoutDefaultsFromPrincipal(
true);
}

void PermissionManager::MaybeCompleteShutdown() {
MOZ_ASSERT(NS_IsMainThread());

DebugOnly<nsresult> rv = GetShutdownPhase()->RemoveBlocker(this);
MOZ_ASSERT(NS_SUCCEEDED(rv));
}

// Async shutdown blocker methods

NS_IMETHODIMP PermissionManager::GetName(nsAString& aName) {
aName = u"PermissionManager: Flushing data"_ns;
return NS_OK;
}

NS_IMETHODIMP PermissionManager::BlockShutdown(
nsIAsyncShutdownClient* aClient) {
RemoveIdleDailyMaintenanceJob();
RemoveAllFromMemory();
CloseDB(eShutdown);

gPermissionManager = nullptr;
return NS_OK;
}

NS_IMETHODIMP
PermissionManager::GetState(nsIPropertyBag** aBagOut) {
nsCOMPtr<nsIWritablePropertyBag2> propertyBag =
do_CreateInstance("@mozilla.org/hash-property-bag;1");

nsresult rv = propertyBag->SetPropertyAsInt32(u"state"_ns, mState);
if (NS_WARN_IF(NS_FAILED(rv))) {
return rv;
}

propertyBag.forget(aBagOut);

return NS_OK;
}

nsCOMPtr<nsIAsyncShutdownClient> PermissionManager::GetShutdownPhase() const {
nsresult rv;
nsCOMPtr<nsIAsyncShutdownService> svc =
do_GetService("@mozilla.org/async-shutdown-service;1", &rv);
MOZ_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv));

nsCOMPtr<nsIAsyncShutdownClient> client;
rv = svc->GetProfileBeforeChange(getter_AddRefs(client));
MOZ_DIAGNOSTIC_ASSERT(NS_SUCCEEDED(rv));

return client;
}

} // namespace mozilla
17 changes: 14 additions & 3 deletions extensions/permissions/PermissionManager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
#define mozilla_PermissionManager_h

#include "nsIPermissionManager.h"
#include "nsIAsyncShutdown.h"
#include "nsIObserver.h"
#include "nsWeakReference.h"
#include "nsCOMPtr.h"
Expand Down Expand Up @@ -46,7 +47,8 @@ class ContentChild;

class PermissionManager final : public nsIPermissionManager,
public nsIObserver,
public nsSupportsWeakReference {
public nsSupportsWeakReference,
public nsIAsyncShutdownBlocker {
friend class dom::ContentChild;

public:
Expand Down Expand Up @@ -158,6 +160,7 @@ class PermissionManager final : public nsIPermissionManager,
NS_DECL_THREADSAFE_ISUPPORTS
NS_DECL_NSIPERMISSIONMANAGER
NS_DECL_NSIOBSERVER
NS_DECL_NSIASYNCSHUTDOWNBLOCKER

PermissionManager();
static already_AddRefed<nsIPermissionManager> GetXPCOMSingleton();
Expand Down Expand Up @@ -445,8 +448,12 @@ class PermissionManager final : public nsIPermissionManager,
void NotifyObservers(nsIPermission* aPermission, const char16_t* aData);

// Finalize all statements, close the DB and null it.
// if aRebuildOnSuccess, reinitialize database
void CloseDB(bool aRebuildOnSuccess = false);
enum CloseDBNextOp {
eNone,
eRebuldOnSuccess,
eShutdown,
};
void CloseDB(CloseDBNextOp aNextOp);

nsresult RemoveAllInternal(bool aNotifyObservers);
nsresult RemoveAllFromMemory();
Expand Down Expand Up @@ -483,6 +490,10 @@ class PermissionManager final : public nsIPermissionManager,
uint32_t aExpireType, int64_t aExpireTime,
int64_t aModificationTime, int64_t aId);

nsCOMPtr<nsIAsyncShutdownClient> GetShutdownPhase() const;

void MaybeCompleteShutdown();

nsRefPtrHashtable<nsCStringHashKey, GenericNonExclusivePromise::Private>
mPermissionKeyPromiseMap;

Expand Down

0 comments on commit 8a5c2d5

Please sign in to comment.