Bug 1350637 - Part 11: Fix a race between parent/child process; r=asuth

When a storage child actor gets the xpcom-shutdown notification, it releases
the actor singleton which asynchronously sends __delete__ to the parent and
destroys the child actor immediatelly. However, at the same time parent can be
sending a message to the child which results in a routing error since the child
actor doesn't exist anymore. The routing error causes a crash.
This patch prevents the race by doing the teardown in two steps. We send a
message to the parent DeleteMe and parent then sends __delete__ to the child.
This commit is contained in:
Jan Varga 2017-08-08 23:02:57 +02:00
parent 8e40043ab4
commit bd9fcfbc01
3 changed files with 21 additions and 25 deletions

View File

@ -22,7 +22,7 @@ sync protocol PBackgroundStorage
manager PBackground;
parent:
async __delete__();
async DeleteMe();
sync Preload(nsCString originSuffix,
nsCString originNoSuffix,
@ -52,6 +52,8 @@ parent:
async ClearMatchingOriginAttributes(OriginAttributesPattern pattern);
child:
async __delete__();
async Observe(nsCString topic,
nsString originAttributesPattern,
nsCString originScope);

View File

@ -13,6 +13,7 @@
#include "mozilla/ipc/BackgroundChild.h"
#include "mozilla/ipc/BackgroundParent.h"
#include "mozilla/ipc/PBackgroundChild.h"
#include "mozilla/ipc/PBackgroundParent.h"
#include "mozilla/Unused.h"
#include "nsIDiskSpaceWatcher.h"
#include "nsThreadUtils.h"
@ -52,25 +53,6 @@ private:
}
};
NS_IMPL_ADDREF(StorageDBChild)
NS_IMETHODIMP_(MozExternalRefCountType) StorageDBChild::Release(void)
{
NS_PRECONDITION(0 != mRefCnt, "dup release");
nsrefcnt count = --mRefCnt;
NS_LOG_RELEASE(this, count, "StorageDBChild");
if (count == 1 && mIPCOpen) {
Send__delete__(this);
return 0;
}
if (count == 0) {
mRefCnt = 1;
delete this;
return 0;
}
return count;
}
void
StorageDBChild::AddIPDLReference()
{
@ -405,6 +387,8 @@ ShutdownObserver::Observe(nsISupports* aSubject,
if (sStorageChild) {
sStorageChildDown = true;
MOZ_ALWAYS_TRUE(sStorageChild->PBackgroundStorageChild::SendDeleteMe());
NS_RELEASE(sStorageChild);
sStorageChild = nullptr;
}
@ -601,6 +585,18 @@ StorageDBParent::ActorDestroy(ActorDestroyReason aWhy)
// Implement me! Bug 1005169
}
mozilla::ipc::IPCResult
StorageDBParent::RecvDeleteMe()
{
AssertIsOnBackgroundThread();
IProtocol* mgr = Manager();
if (!PBackgroundStorageParent::Send__delete__(this)) {
return IPC_FAIL_NO_REASON(mgr);
}
return IPC_OK();
}
mozilla::ipc::IPCResult
StorageDBParent::RecvAsyncPreload(const nsCString& aOriginSuffix,
const nsCString& aOriginNoSuffix,

View File

@ -44,8 +44,7 @@ public:
static StorageDBChild*
GetOrCreate();
NS_IMETHOD_(MozExternalRefCountType) AddRef(void);
NS_IMETHOD_(MozExternalRefCountType) Release(void);
NS_INLINE_DECL_REFCOUNTING(StorageDBChild);
void AddIPDLReference();
void ReleaseIPDLReference();
@ -111,9 +110,6 @@ private:
nsTHashtable<nsCStringHashKey>& OriginsHavingData();
ThreadSafeAutoRefCnt mRefCnt;
NS_DECL_OWNINGTHREAD
// Held to get caches to forward answers to.
RefPtr<LocalStorageManager> mManager;
@ -229,6 +225,8 @@ public:
private:
// IPC
virtual void ActorDestroy(ActorDestroyReason aWhy) override;
mozilla::ipc::IPCResult RecvDeleteMe() override;
mozilla::ipc::IPCResult RecvAsyncPreload(const nsCString& aOriginSuffix,
const nsCString& aOriginNoSuffix,
const bool& aPriority) override;