1905011 - Replace counting of pending clear/shutdown storage operations with evaluation of existing directory locks

Status: RESOLVED FIXED

(Core :: Storage: Quota Manager, task, P2)

Description

[Jan Varga [:janv]]

Counting of clear/shutdown storage operations was only a temporary solution. We are now approaching a point when we will be able to initialize origins asynchronously and that can't efficiently work with counting of clear origin operations because initialization and clearing of origins is parameterized by persistence type and actual origin. So there can be a clear origin operation which doesn't block other init origin operation. Couting of clear operations would be problematic in that case. Evaluation of existing directory locks should work both for storage and origin initialization.

Comment 1

[Jan Varga [:janv]]

Attachment: Bug 1905011 - Add callbacks for origin operations; r=#dom-storage

Some gtests need to invoke functions at specifix moments, for example just
before an origin operation is finished. This patch adds support for that and
also callbacks which are resolved/rejected just after an origin operation is
finished. Consumers can either use asynchronous or synchronous callbacks.

Comment 2

[Jan Varga [:janv]]

Attachment: Bug 1905011 - Add optional callbacks to QuotaManager::ShutdownStorage; r=#dom-storage

Comment 3

[Jan Varga [:janv]]

Attachment: Bug 1905011 - Add testing for InitializeStorage scheduled just before ShutdownStorage is finished; r=#dom-storage

This is an edge case and was the primary reason for making DirectoryLock::Drop
async. Async Drop is needed for replacing counting of pending clear/shutdown storage operations with evaluation of existing directory locks because of this
scenario:

• ShutdownStorageOp::DoDirectoryWork is done
• NormalOriginOperationBase::UnblockOpen calls SendResults
• ResolvableNormalOriginOp::SendResults resolves the promise (a new runnable is
  dispatched to the current thread)
• NormalOriginOperationBase::UnblockOpen calls CloseDirectory
• ShutdownStorageOp::CloseDirectory drops the directory lock immediately and
  synchronously
• The current runnable is done
• A pending runnable is being processed which calls InitializeStorage
• InitializeStorage still sees that mStorageInitializes is true and that there
  are no existing directory locks, so it immediately resolves its promise
• The runnable dispatched from ResolvableNormalOriginOp::SendResults is
  processed and the handler for it sets mStorageInitialized to false in
  QuotaManager::ShutdownStorage
• Finally, mStorageInitialized is false, but it should be true

So the problem in the scenario is that the directory lock is dropped
synchronously. However, all directory locks are now dropped explicitly and
asynchronously so InitializeStorage would still see the directory lock for
ShutdownStorage and wouldn't immediately resolve its promise.

This new test simulates the described scenario.

Comment 4

[Jan Varga [:janv]]

Attachment: Bug 1905011 - Remove unneeded checks involving mShutdownStorageOpCount; r=#dom-storage

QuotaManager::InitializeStorage and QuotaManager::InitializeTemporaryStorage
currently check if there are any clear or shutdown storage operations even when
the directory lock has been acquired. There should be no such operations, so
the check can be removed.

Comment 5

[Jan Varga [:janv]]

Attachment: Bug 1905011 - Replace counting of pending clear/shutdown storage operations with evaluation of existing directory locks; r=#dom-storage

Counting of clear/shutdown storage operations was only a temporary solution.
We are now approaching a point when we will be able to initialize origins
asynchronously and that can't efficiently work with counting of clear origin
operations because initialization and clearing of origins is parameterized by
persistence type and actual origin. So there can be a clear origin operation
which doesn't block other init origin operation. Couting of clear operations
would be problematic in that case.
Evaluation of existing directory locks should work both for storage and origin
initialization.

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1bf9d6c5fe67
Add callbacks for origin operations; r=dom-storage-reviewers,jari
https://hg.mozilla.org/integration/autoland/rev/1622d140f4f0
Add optional callbacks to QuotaManager::ShutdownStorage; r=dom-storage-reviewers,jari
https://hg.mozilla.org/integration/autoland/rev/ebd5ed5b9adc
Add testing for InitializeStorage scheduled just before ShutdownStorage is finished; r=dom-storage-reviewers,jari

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3441b96db23e
Remove unneeded checks involving mShutdownStorageOpCount; r=hsingh

Comment 8

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/1bf9d6c5fe67
https://hg.mozilla.org/mozilla-central/rev/1622d140f4f0
https://hg.mozilla.org/mozilla-central/rev/ebd5ed5b9adc
https://hg.mozilla.org/mozilla-central/rev/3441b96db23e

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/df5e2759f42a
Replace counting of pending clear/shutdown storage operations with evaluation of existing directory locks; r=dom-storage-reviewers,jari

Comment 10

[Cristina Horotan [:chorotan]]

https://hg.mozilla.org/mozilla-central/rev/df5e2759f42a