Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2024-11-23 04:41:11 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1928939 - LSNG: Allow one single prepare datastore operation to block multiple prepare datastore operations; r=dom-storage-reviewers,asuth

Browse Source 
Bug 1919788 introduced calling OpenClientDirectory as a first step, which made
it possible for PrepareDatastoreOp::CheckExistingOperations to not be called in
the same order as the operations were created. If OpenClientDirectory is called
multiple times with the same arguments, the promises can be resolved/rejected
in random order. This issue became even more apparent after bug 1866402, which
added origin initialization to OpenClientDirectory.

The serialization based on mDelayedOp can only reliably work if
CheckExistingOperations is invoked in the same order as operations are created.
Therefore, the operations now utilize a more sophisticated method to track
dependencies.

This fix implements a robust dependency tracking mechanism using mBlocking and
mBlockedOn lists, ensuring that each PrepareDatastoreOp proceeds in a
serialized manner based on its dependencies, thereby maintaining the correct
order of operations and enhancing stability in datastore preparations.

Differential Revision: https://phabricator.services.mozilla.com/D227985
This commit is contained in:
Jan Varga 2024-11-05 18:46:40 +00:00
parent c11b49367c
commit 689e3495aa
2 changed files with 39 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
dom/localstorage/ActorsParent.cpp
View File

@ -2202,8 +2202,29 @@ class LSRequestBase : public DatastoreOperationBase,
mozilla::ipc::IPCResult RecvFinish() final;
};
template <typename T>
class SerializedOp {
protected:
void AddBlockingOp(T& aOp) { mBlocking.AppendElement(WrapNotNull(&aOp)); }
void AddBlockedOnOp(T& aOp) { mBlockedOn.AppendElement(WrapNotNull(&aOp)); }
void MaybeUnblock(T& aOp) {
mBlockedOn.RemoveElement(&aOp);
if (mBlockedOn.IsEmpty()) {
Unblock();
}
}
virtual void Unblock() = 0;
nsTArray<NotNull<RefPtr<T>>> mBlocking;
nsTArray<NotNull<RefPtr<T>>> mBlockedOn;
};
class PrepareDatastoreOp
: public LSRequestBase,
public SerializedOp<PrepareDatastoreOp>,
public SupportsCheckedUnsafePtr<CheckIf<DiagnosticAssertEnabled>> {
class LoadDataOp;
@ -2257,7 +2278,6 @@ class PrepareDatastoreOp
};
mozilla::glean::TimerId mProcessingTimerId;
RefPtr<PrepareDatastoreOp> mDelayedOp;
RefPtr<ClientDirectoryLock> mPendingDirectoryLock;
RefPtr<ClientDirectoryLock> mDirectoryLock;
RefPtr<ClientDirectoryLock> mExtraDirectoryLock;
@ -2340,6 +2360,10 @@ class PrepareDatastoreOp
nsresult OpenDirectory();
void Unblock() override {
MOZ_ALWAYS_SUCCEEDS(NS_DispatchToCurrentThread(this));
}
nsresult CheckExistingOperations();
nsresult CheckClosingDatastoreInternal();
@ -6673,18 +6697,10 @@ void PrepareDatastoreOp::Log() {
switch (mNestedState) {
case NestedState::CheckClosingDatastore: {
for (uint32_t index = gPrepareDatastoreOps->Length(); index > 0;
index--) {
const auto& existingOp = (*gPrepareDatastoreOps)[index - 1];
for (const auto& blockedOn : mBlockedOn) {
LS_LOG((" blockedOn: [%p]", blockedOn.get().get()));
if (existingOp->mDelayedOp == this) {
LS_LOG((" mDelayedBy: [%p]",
static_cast<PrepareDatastoreOp*>(existingOp.get())));
existingOp->Log();
break;
}
blockedOn->Log();
}
break;
@ -6848,6 +6864,8 @@ nsresult PrepareDatastoreOp::CheckExistingOperations() {
// See if this PrepareDatastoreOp needs to wait.
bool foundThis = false;
bool blocked = false;
for (uint32_t index = gPrepareDatastoreOps->Length(); index > 0; index--) {
const auto& existingOp = (*gPrepareDatastoreOps)[index - 1];
@ -6857,15 +6875,15 @@ nsresult PrepareDatastoreOp::CheckExistingOperations() {
}
if (foundThis && existingOp->Origin() == Origin()) {
// Only one op can be delayed.
MOZ_ASSERT(!existingOp->mDelayedOp);
existingOp->mDelayedOp = this;
return NS_OK;
existingOp->AddBlockingOp(*this);
AddBlockedOnOp(*existingOp);
blocked = true;
}
}
QM_TRY(MOZ_TO_RESULT(CheckClosingDatastoreInternal()));
if (!blocked) {
QM_TRY(MOZ_TO_RESULT(CheckClosingDatastoreInternal()));
}
return NS_OK;
}
@ -7677,9 +7695,10 @@ void PrepareDatastoreOp::ConnectionClosedCallback() {
void PrepareDatastoreOp::CleanupMetadata() {
AssertIsOnOwningThread();
if (mDelayedOp) {
MOZ_ALWAYS_SUCCEEDS(NS_DispatchToCurrentThread(mDelayedOp.forget()));
for (const NotNull<RefPtr<PrepareDatastoreOp>>& blockingOp : mBlocking) {
blockingOp->MaybeUnblock(*this);
}
mBlocking.Clear();
MOZ_ASSERT(gPrepareDatastoreOps);
gPrepareDatastoreOps->RemoveElement(this);

1
dom/localstorage/test/unit/xpcshell.toml
View File

@ -82,7 +82,6 @@ support-files = ["migration_emptyValue_profile.zip",]
["test_old_lsng_pref.js"]
["test_open_and_multiple_preloads.js"]
skip-if = ["true"]
["test_orderingAfterRemoveAdd.js"]