Bug 1605973 Comment 27 Edit History

Note: The actual edited comment in the bug view page will always show the original commenter’s name and original timestamp.

(In reply to Jan Varga [:janv] from comment #25)
> it's thread 17 and `CacheQuotaClient::GetUsageForOriginInternal` is on the stack. That method has a loop which traverses over directory entries on disk.

Can we avoid doing this for system principal consumers like remote settings? QM restrictions shouldn't apply to our indexeddb use...

> There's a check `QuotaManager::IsShuttingDown` in the loop, but the real problem here seems to be that QuotaManager hasn't been notified about the shutdown yet so `QuotaManager::IsShuttingDown` returns false probably.
> I assume that remote settings has a shutdown blocker for `profile-before-change`.

Yes.

> Our current design is that we let all consumers to finish/cleanup after getting profile-before-change, then we notify QuotaManager (profile-before-change-qm) which sets the isShuttingDown flag, so some pending initialization can be interrupted.
> 
> Maybe we need another notification for QuotaManaget which is sent before profile-before-change, the order would then be:

This would help a bit for the current situation, though if the QM stuff only happens when we commit a transaction, it may not be sufficient - after all, it's quite possible for a transaction to start before whatever cut-off point we pick to stop accepting new connections to idb, and then run the expensive QM stuff later.

It seems the third crash-report I linked in my comment, ie https://crash-stats.mozilla.org/report/index/f45b886a-1b06-480a-bbd0-6f8460200506#allthreads , is stuck doing the actual sqlite transaction work. I haven't audited the full set of crashreports to see how common this is; the crash-stats UI doesn't make this all that easy, but if it's useful to do this I can take a look?
(In reply to Jan Varga [:janv] from comment #25)
> it's thread 17 and `CacheQuotaClient::GetUsageForOriginInternal` is on the stack. That method has a loop which traverses over directory entries on disk.

Can we avoid doing this for system principal consumers like remote settings? QM restrictions shouldn't apply to our indexeddb use...

> There's a check `QuotaManager::IsShuttingDown` in the loop, but the real problem here seems to be that QuotaManager hasn't been notified about the shutdown yet so `QuotaManager::IsShuttingDown` returns false probably.
> I assume that remote settings has a shutdown blocker for `profile-before-change`.

Yes.

> Our current design is that we let all consumers to finish/cleanup after getting profile-before-change, then we notify QuotaManager (profile-before-change-qm) which sets the isShuttingDown flag, so some pending initialization can be interrupted.
> 
> Maybe we need another notification for QuotaManaget which is sent before profile-before-change, the order would then be:

This would help a bit for the current situation, though if some of the expensive stuff only happens when we commit a transaction, it may not be sufficient - after all, it's quite possible for a transaction to start before whatever cut-off point we pick to stop accepting new connections to idb, and then run the expensive stuff later.

It seems the third crash-report I linked in my comment, ie https://crash-stats.mozilla.org/report/index/f45b886a-1b06-480a-bbd0-6f8460200506#allthreads , is stuck doing the actual sqlite transaction work. I haven't audited the full set of crashreports to see how common this is; the crash-stats UI doesn't make this all that easy, but if it's useful to do this I can take a look?

Back to Bug 1605973 Comment 27