1923056 - Crash in [@ mozilla::dom::quota::DirectoryLockImpl::~DirectoryLockImpl]

Status: RESOLVED FIXED

(Core :: Storage: Quota Manager, defect, P1)

Description

[Gabriele Svelto [:gsvelto]]

Crash report: https://crash-stats.mozilla.org/report/index/91126153-b87a-41b3-919c-171b70241006

MOZ_CRASH Reason:

MOZ_DIAGNOSTIC_ASSERT(!mRegistered)

Top 10 frames:

0  xul.dll  mozilla::dom::quota::DirectoryLockImpl::~DirectoryLockImpl()  dom/quota/DirectoryLockImpl.cpp:72
1  xul.dll  mozilla::dom::quota::DirectoryLockImpl::Release()  dom/quota/DirectoryLockImpl.h:185
2  xul.dll  mozilla::RefPtrTraits<mozilla::dom::quota::DirectoryLock>::Release(mozilla::d...  mfbt/RefPtr.h:49
2  xul.dll  RefPtr<mozilla::dom::quota::DirectoryLock>::ConstRemovingRefPtrTraits<mozilla...  mfbt/RefPtr.h:409
2  xul.dll  RefPtr<mozilla::dom::quota::DirectoryLock>::~RefPtr()  mfbt/RefPtr.h:80
2  xul.dll  mozilla::dom::(anonymous namespace)::PrepareDatastoreOp::~PrepareDatastoreOp()  dom/localstorage/ActorsParent.cpp:6578
2  xul.dll  mozilla::dom::(anonymous namespace)::PrepareDatastoreOp::~PrepareDatastoreOp()  dom/localstorage/ActorsParent.cpp:6573
3  xul.dll  mozilla::Runnable::Release()  xpcom/threads/nsThreadUtils.cpp:66
3  xul.dll  mozilla::InputStreamLengthHelper::Release()  storage/mozStorageConnection.cpp:778
4  xul.dll  mozilla::RefPtrTraits<mozilla::dom::(anonymous namespace)::LSRequestBase>::Re...  dom/localstorage/ActorsParent.cpp:3448

This appears to be a different issue from the older bug 1903387. It only affects Windows for the time being, but this could just be because we have more Windows users on nightly than on the other platforms.

Comment 1

[Jan Varga [:janv]]

It seems this started happening after bug 1919788.

Comment 2

[Jan Varga [:janv]]

Attachment: Bug 1923056 - Improve assertions in PrepareDatastoreOp destructor; r=#dom-storage

We know that in some cases a directory lock isn't being dropped in
PrepareDatastoreOp::Cleanup, but it's unclear if it's mDirectoryLock or
mExtraDirectoryLock. This patch should help identify that.

Comment 3

[Jan Varga [:janv]]

Attachment: Bug 1923056 - Speculative fix; r=#dom-storage

Comment 4

[Jan Varga [:janv]]

I think I found a code path which may be causing the assertion, working on a test and fix.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a6bc6ec5b8f4
Improve assertions in PrepareDatastoreOp destructor; r=dom-storage-reviewers,asuth

Comment 6

[Jan Varga [:janv]]

Attachment: Bug 1923056 - Add a new test for shutdown requested after starting request finalization; r=#dom-storage

Comment 7

[Jan Varga [:janv]]

Attachment: Bug 1923056 - Transform regular directory lock into extra directory lock immediately after finding existing datastore; r=#dom-storage

PrepareDatastoreOp::GetResponse is not called if there was an error, so the
transformation of the regular directory lock into the extra directory lock must
be done right after finding existing datastore.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ba87a3d5133a
Add a new test for shutdown requested after starting request finalization; r=dom-storage-reviewers,asuth
https://hg.mozilla.org/integration/autoland/rev/d4f92abdacdc
Transform regular directory lock into extra directory lock immediately after finding existing datastore; r=dom-storage-reviewers,asuth

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

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

Comment 10

[Serban Stanca [:SerbanS]]

https://hg.mozilla.org/mozilla-central/rev/ba87a3d5133a
https://hg.mozilla.org/mozilla-central/rev/d4f92abdacdc