1450160 - Firefox Crashes when ServiceWorker uses IndexedDB and IDB not available [Crash in IPCError-browser | IndexedDB CheckPermission 1]

Status: RESOLVED WORKSFORME

(Core :: Storage: IndexedDB, defect, P3)

Description

[charlie.croom]

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_3) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3325.181 Safari/537.36

Steps to reproduce:

1) clear all data
2) set dom.indexedDB.enabled to false
3) visit mobile.twitter.com


Actual results:

Either the page will not load (if you've never been before) or the tab will crash (if you've been and the service worker is installed.

If the site doesn't load fully, then you are getting "InvalidStateError: A mutation operation was attempted on a database that did not allow mutations.", this is because when indexedDB is turned off, even accessing window.indexedDB will throw an exception. (Instead of just setting window.indexedDB to null).

That's handleable with a simple try/catch.

Now if you properly handle that, the site will load and after the SW is installed, the tab will immediately crash. This is because the SW also uses an interface to IDB. However, in the SW scope self.indexedDB is defined as IDBFactory, so detection of IDB being available passes.

When the first call is then made to IDB, the SW (and site along with it) will crash. This is not an uncaught exception, but rather a full crash.


Expected results:

Accessing self.indexedDB in the ServiceWorker should throw the same exception as window.indexedDB in the normal DOM. Or, when unavailable, both should simply be set to null for easier detection.

Alternatively, some other mechanism for detecting IDB availability should be made available.

Further, crashes in the SW should not crash the main tab (though, maybe there's some overlap). Instead, it should simply stop and remove the ServiceWorker, or maybe throw an error in the main window (SWCrashed) for error logging

Comment 1

[charlie.croom]

Potentially related to https://bugzilla.mozilla.org/show_bug.cgi?id=1320027

Comment 2

[Marion Daly [:mdaly]]

Ben: P3 for now unless confirmed and you feel this bug has any serious implications.

Comment 3

[Ben Kelly [:bkelly, not reviewing]]

Andrew, is dom.indexeddb.enabled=false something we actually support for our users?  Why does this exist?  The browser itself uses IDB.

Comment 4

[Ben Kelly [:bkelly, not reviewing]]

I can't comprehend why anyone would want to disable IDB, but leave service workers enabled.

Comment 5

[Andrew Sutherland [:asuth] (he/him)]

## Short story:
- I assume the pref is a legacy of the original implementation of indexedDB.  A simpler time, when LocalStorage roamed the land, synchronously blocking the main thread whenever it pleased.
- The pref is listed in all.js so users can find it in about:config.
- The pref doesn't affect the system principal due to explicit opt-outs in the various checks.
- In yet another main-thread/worker asymmetry, we don't check the pref when workers are involved.
- In the OpenDatabaseOp, for content principals, we'll kill the ContentParent (if we're e10s) if it sent a request, because obviously the content process has been hacked.

We can probably remove the pref and direct anyone who was using the pref to instead disable storage permissions at a global level by using "Block cookies and site data".

### Details:
These are where we check the pref:

- IDBFactory::CreateForWindow()[1]: System principal ignores the pref.
- IDBFactory::CreateForMainThreadJSInternal()[2]: System principal ignores the pref.  This is the mechanism by which Sandboxes and JSM's and non-window Chrome JS get access to IndexedDB using Cu.importGlobalProperties(["indexedDB"]) and Cu.Sandbox(principal, { wantGlobalProperties: ["indexedDB"] }).
- FactoryOp::CheckPermission()[3]: Kill content processes that try and use IndexedDB from a content principal if the pref is disabled.

1: https://searchfox.org/mozilla-central/rev/57bbc1ac58816dc054df242948f3ecf75e12df5f/dom/indexedDB/IDBFactory.cpp#109
2: https://searchfox.org/mozilla-central/rev/57bbc1ac58816dc054df242948f3ecf75e12df5f/dom/indexedDB/IDBFactory.cpp#227
3: https://searchfox.org/mozilla-central/rev/57bbc1ac58816dc054df242948f3ecf75e12df5f/dom/indexedDB/ActorsParent.cpp#20970

Comment 6

[Andrew Sutherland [:asuth] (he/him)]

And to explicitly address comment 0 and the crash, what's happening is:
- The pref doesn't disable IDB in the workers because we don't check the pref there.
- When the worker tries to use IDB, its attempt to open a database will cause its containing process to be terminated by the parent process because of the illegal use of IDB by the child process.
- There's no way for the worker to detect the problem without asking a page context to check whether it's allowed to use IDB.

Comment 7

[Chris Peterson [:cpeterson]]

Here are some crash IDs from Charlie. Unfortunately, the stacks don't have symbols:

CSignatureTool: No proper signature could be created because no good data for the crashing thread (0) was found; Signature replaced with an IPC Channel Error, was: "@0x7fff7daac7c2"

bp-6fde1386-d4f9-4492-aa1d-921fd0180330
bp-a15e9c49-4217-4cb6-9713-dc4ab0180330
bp-53677257-6b9c-4114-976b-5d7d80180330
bp-e678b73f-671b-4438-aad6-4e4670180330
bp-e87977fc-79b7-46a3-86ce-21bb00180330
bp-6aa37ece-0d36-46b8-ba24-d8ea40180330
bp-d0e23fe2-0627-44ea-a7db-ecd1b0180330
bp-4d33c3de-b038-49d7-816c-d8ead0180330
bp-68a79133-7f01-4446-a7e5-df0ff0180330
bp-78d4b27d-1e59-47df-b818-00ea41180329
bp-8a756aaa-3a5e-4b65-895d-eb7161180329

Comment 8

[Ben Kelly [:bkelly, not reviewing]]

BTW, Charlie indicated in a separate email that he is only using the pref to try to simulate the problem.

Comment 9

[Ben Kelly [:bkelly, not reviewing]]

AFAICT this is more of an IDB integration issue with workers in general.  We need to check the preference in here:

https://searchfox.org/mozilla-central/source/dom/indexedDB/IDBFactory.cpp#189

Comment 12

[Andrew McCreight [:mccr8]]

90 of these crashes from a single installation in Nightly. For instance: bp-9fe73ef8-f4f6-4584-a053-f33ec0190815

Various YouTube URLs.

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

Closing because no crashes reported for 12 weeks.

Comment 14

[Andrew Sutherland [:asuth] (he/him)]

Presumably this was largely addressed by bug 1488583 removing the obsolete preference in Fx72 and ESR68 going away ~12 weeks ago.