Prevent potential leak of IndexedDB mDatabaseActor

This commit is contained in:
Amelia Hart 2023-06-29 19:58:27 +02:00 • committed by wuggy
commit a583989bb4
2 changed files with 42 additions and 0 deletions

View file

@ -1415,6 +1415,7 @@ BackgroundFactoryRequestChild::BackgroundFactoryRequestChild(
uint64_t aRequestedVersion) uint64_t aRequestedVersion)
: BackgroundRequestChildBase(aOpenRequest) : BackgroundRequestChildBase(aOpenRequest)
, mFactory(aFactory) , mFactory(aFactory)
, mDatabaseActor(nullptr)
, mRequestedVersion(aRequestedVersion) , mRequestedVersion(aRequestedVersion)
, mIsDeleteOp(aIsDeleteOp) , mIsDeleteOp(aIsDeleteOp)
{ {
@ -1439,6 +1440,15 @@ BackgroundFactoryRequestChild::GetOpenDBRequest() const
return static_cast<IDBOpenDBRequest*>(mRequest.get()); return static_cast<IDBOpenDBRequest*>(mRequest.get());
} }
void
BackgroundFactoryRequestChild::SetDatabaseActor(BackgroundDatabaseChild* aActor)
{
AssertIsOnOwningThread();
MOZ_ASSERT(!aActor || !mDatabaseActor);
mDatabaseActor = aActor;
}
bool bool
BackgroundFactoryRequestChild::HandleResponse(nsresult aResponse) BackgroundFactoryRequestChild::HandleResponse(nsresult aResponse)
{ {
@ -1450,6 +1460,11 @@ BackgroundFactoryRequestChild::HandleResponse(nsresult aResponse)
DispatchErrorEvent(mRequest, aResponse); DispatchErrorEvent(mRequest, aResponse);
if (mDatabaseActor) {
mDatabaseActor->ReleaseDOMObject();
MOZ_ASSERT(!mDatabaseActor);
}
return true; return true;
} }
@ -1468,6 +1483,7 @@ BackgroundFactoryRequestChild::HandleResponse(
IDBDatabase* database = databaseActor->GetDOMObject(); IDBDatabase* database = databaseActor->GetDOMObject();
if (!database) { if (!database) {
databaseActor->EnsureDOMObject(); databaseActor->EnsureDOMObject();
MOZ_ASSERT(mDatabaseActor);
database = databaseActor->GetDOMObject(); database = databaseActor->GetDOMObject();
MOZ_ASSERT(database); MOZ_ASSERT(database);
@ -1475,6 +1491,8 @@ BackgroundFactoryRequestChild::HandleResponse(
MOZ_ASSERT(!database->IsClosed()); MOZ_ASSERT(!database->IsClosed());
} }
MOZ_ASSERT(mDatabaseActor == databaseActor);
if (database->IsClosed()) { if (database->IsClosed()) {
// If the database was closed already, which is only possible if we fired an // If the database was closed already, which is only possible if we fired an
// "upgradeneeded" event, then we shouldn't fire a "success" event here. // "upgradeneeded" event, then we shouldn't fire a "success" event here.
@ -1487,6 +1505,7 @@ BackgroundFactoryRequestChild::HandleResponse(
} }
databaseActor->ReleaseDOMObject(); databaseActor->ReleaseDOMObject();
MOZ_ASSERT(!mDatabaseActor);
return true; return true;
} }
@ -1507,6 +1526,8 @@ BackgroundFactoryRequestChild::HandleResponse(
DispatchSuccessEvent(&helper, successEvent); DispatchSuccessEvent(&helper, successEvent);
MOZ_ASSERT(!mDatabaseActor);
return true; return true;
} }
@ -1737,6 +1758,8 @@ BackgroundDatabaseChild::EnsureDOMObject()
mDatabase = mTemporaryStrongDatabase; mDatabase = mTemporaryStrongDatabase;
mSpec.forget(); mSpec.forget();
mOpenRequestActor->SetDatabaseActor(this);
} }
void void
@ -1748,6 +1771,8 @@ BackgroundDatabaseChild::ReleaseDOMObject()
MOZ_ASSERT(mOpenRequestActor); MOZ_ASSERT(mOpenRequestActor);
MOZ_ASSERT(mDatabase == mTemporaryStrongDatabase); MOZ_ASSERT(mDatabase == mTemporaryStrongDatabase);
mOpenRequestActor->SetDatabaseActor(nullptr);
mOpenRequestActor = nullptr; mOpenRequestActor = nullptr;
// This may be the final reference to the IDBDatabase object so we may end up // This may be the final reference to the IDBDatabase object so we may end up

View file

@ -254,6 +254,20 @@ class BackgroundFactoryRequestChild final
friend class PermissionRequestParent; friend class PermissionRequestParent;
RefPtr<IDBFactory> mFactory; RefPtr<IDBFactory> mFactory;
// Normally when opening of a database is successful, we receive a database
// actor in request response, so we can use it to call ReleaseDOMObject()
// which clears temporary strong reference to IDBDatabase.
// However, when there's an error, we don't receive a database actor and
// IDBRequest::mTransaction is already cleared (must be). So the only way how
// to call ReleaseDOMObject() is to have a back-reference to database actor.
// This creates a weak ref cycle between
// BackgroundFactoryRequestChild (using mDatabaseActor member) and
// BackgroundDatabaseChild actor (using mOpenRequestActor member).
// mDatabaseActor is set in EnsureDOMObject() and cleared in
// ReleaseDOMObject().
BackgroundDatabaseChild* mDatabaseActor;
const uint64_t mRequestedVersion; const uint64_t mRequestedVersion;
const bool mIsDeleteOp; const bool mIsDeleteOp;
@ -271,6 +285,9 @@ private:
// Only destroyed by BackgroundFactoryChild. // Only destroyed by BackgroundFactoryChild.
~BackgroundFactoryRequestChild(); ~BackgroundFactoryRequestChild();
void
SetDatabaseActor(BackgroundDatabaseChild* aActor);
bool bool
HandleResponse(nsresult aResponse); HandleResponse(nsresult aResponse);