1181871 - Quote Manager initialization will bail out if it finds a directory it doesn't know about

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[(no longer active)]

I found this while debugging bug 1180765.

STR:
1. Add a directory to profiledir/storage/default/https+++medium.com.  Mine was called "copy" but the name doesn't matter as long as QM doesn't know it.
2. Load medium.com.

In QuotaManager::InitializeOrigin(), we fail hard if we find a directory name that we don't know about: <https://dxr.allizom.org/mozilla-central/source/dom/quota/QuotaManager.cpp#2913>

That can result in the following disaster when we are initialing the cache:

[Parent 10250] WARNING: Unknown Cache directory found!: file /Users/ehsan/moz/src/dom/cache/QuotaClient.cpp, line 130
[Parent 10250] WARNING: Unknown directory found!: file /Users/ehsan/moz/src/dom/quota/QuotaManager.cpp, line 2912
[Parent 10250] WARNING: 'NS_FAILED(rv)', file /Users/ehsan/moz/src/dom/quota/QuotaManager.cpp, line 2806
[Parent 10250] WARNING: 'NS_FAILED(rv)', file /Users/ehsan/moz/src/dom/quota/QuotaManager.cpp, line 3262
[Parent 10250] WARNING: 'aRv.Failed()', file /Users/ehsan/moz/src/dom/cache/CacheOpParent.cpp, line 167
Assertion failure: mTarget == NS_GetCurrentThread(), at /Users/ehsan/moz/src/dom/cache/Context.cpp:88
#01: mozilla::dom::cache::Context::Data::~Data()[/Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2cfc925]
#02: mozilla::dom::cache::Context::Data::~Data()[/Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2cfc8a5]
#03: mozilla::dom::cache::Context::Data::Release()[/Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2cfc854]
#04: nsRefPtr<mozilla::dom::cache::Context::Data>::~nsRefPtr()[/Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2cfd9e0]
#05: nsRefPtr<mozilla::dom::cache::Context::Data>::~nsRefPtr()[/Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2cf0005]
#06: mozilla::dom::cache::Context::QuotaInitRunnable::~QuotaInitRunnable()[/Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2cfde32]
#07: mozilla::dom::cache::Context::QuotaInitRunnable::~QuotaInitRunnable()[/Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2cf4a75]
[Parent 10250] WARNING: 'aRv.Failed()', file /Users/ehsan/moz/src/dom/cache/CacheOpChild.cpp, line 100
#08: mozilla::dom::cache::Context::QuotaInitRunnable::~QuotaInitRunnable()[/Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2cf4a99]
#09: mozilla::dom::cache::Context::QuotaInitRunnable::Release()[/Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x2cc6d29]
#10: nsCOMPtr<nsIRunnable>::~nsCOMPtr()[/Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0xca8de]
#11: nsCOMPtr<nsIRunnable>::~nsCOMPtr()[/Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0xca875]
#12: nsThread::ProcessNextEvent(bool, bool*)[/Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x19a2c4]
#13: NS_ProcessNextEvent(nsIThread*, bool)[/Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x20de07]
#14: mozilla::ipc::MessagePumpForNonMainThreads::Run(base::MessagePump::Delegate*)[/Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x867d21]
#15: MessageLoop::RunInternal()[/Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7d3105]
#16: MessageLoop::RunHandler()[/Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7d3015]
#17: MessageLoop::Run()[/Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x7d2fbd]
#18: nsThread::ThreadFunc(void*)[/Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/dist/NightlyDebug.app/Contents/MacOS/XUL +0x19873b]
#19: _pt_root[/Users/ehsan/moz/src/obj-ff-clang-plugin.noindex/dist/NightlyDebug.app/Contents/MacOS/libnss3.dylib +0x379397]
#20: _pthread_body[/usr/lib/system/libsystem_pthread.dylib +0x3268]
#21: _pthread_body[/usr/lib/system/libsystem_pthread.dylib +0x31e5]
Process 10250 stopped
* thread #41: tid = 0xa2324, 0x0000000104a8792f XUL`mozilla::dom::cache::Context::Data::~Data(this=0x0000000147c51d20) + 127 at Context.cpp:88, name = 'IPDL Background', stop reason = EXC_BAD_ACCESS (code=1, address=0x0)
    frame #0: 0x0000000104a8792f XUL`mozilla::dom::cache::Context::Data::~Data(this=0x0000000147c51d20) + 127 at Context.cpp:88
   85  	    // Context code should guarantee that we are destroyed on the target
   86  	    // thread.  If we're not, then QuotaManager might race and try to clear the
   87  	    // origin out from under us.
-> 88  	    MOZ_ASSERT(mTarget == NS_GetCurrentThread());
   89  	  }
   90
   91  	  nsCOMPtr<nsIThread> mTarget;

Comment 1

[Ben Kelly [:bkelly, not reviewing]]

In general QuotaManager stuff is unforgiving of unexpected files/directories.  I'm ok loosening that, but it would be inconsistent with IDB, asmjs cache, etc.

Alternatively, we could also just fix the assert here.

What do you prefer?

Comment 2

[(no longer active)]

If the assertion failures show an actual issue, let's fix them.  I didn't look at it in depth so I'm not sure if there is value in fixing it.

As to the general QM issue, I defer to Jan.  Bug 1181887 on the service worker side of things will make us at least able to load the website when this happens.

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

I guess I would prefer to keep the current strictness about directory structure.  It makes reasoning about later code easier.  I'll fix the assertion issue.

Comment 4

[Jan Varga [:janv]]

(In reply to Ben Kelly [:bkelly] from comment #3)
> I guess I would prefer to keep the current strictness about directory
> structure.  It makes reasoning about later code easier.  I'll fix the
> assertion issue.

Good to hear that, I don't want to change these things at this point in time.

Comment 5

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P1 Only enforce Cache Context shared data struction on target thread after init. r=ehsan

Fix the Cache assert to only fire after successful initialization.

Comment 6

[Ben Kelly [:bkelly, not reviewing]]

Attachment: P2 Fix ServiceWorkerManager usage of stack-based ErrorResult. r=ehsan

Local testing showed that we also hit asserts in ServiceWorkerManager due to its incorrect usage of stack-based ErrorResult values.  This fixes those problems.

After these two patches we don't crash any more.  The load still hangs, though, but I believe thats covered under bug 1181887.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=d5a7cc412207

Comment 7

[(no longer active)]

Comment on attachment 8631743 [details] [diff] [review]
P1 Only enforce Cache Context shared data struction on target thread after init. r=ehsan

Review of attachment 8631743 [details] [diff] [review]:
-----------------------------------------------------------------

There is a typo in the commit message!

Comment 8

[(no longer active)]

(In reply to Ben Kelly [:bkelly] from comment #6)
> Local testing showed that we also hit asserts in ServiceWorkerManager due to
> its incorrect usage of stack-based ErrorResult values.  This fixes those
> problems.

Yeah, I actually saw that too since I commented the assert out locally, but forgot to put it in the bug.  Thanks for catching it!

> After these two patches we don't crash any more.  The load still hangs,
> though, but I believe thats covered under bug 1181887.

Agreed.

Comment 9

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/113c98c78a3f
https://hg.mozilla.org/integration/mozilla-inbound/rev/9741eaecc886

Comment 10

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/113c98c78a3f
https://hg.mozilla.org/mozilla-central/rev/9741eaecc886