870951 - VacuumManager asserts when running in a child process.

Status: RESOLVED FIXED

(Core :: SQLite and Embedded Database Bindings, defect)

Description

[Dave Hylands [:dhylands]]

I happened to be running a debug build (trunk) of B2G on my unagi phone and saw the following in my log. It seems that setting a preference isn't allowed in child content processes, and that in turn triggered the assert.

[Child 472] WARNING: NS_ENSURE_TRUE(XRE_GetProcessType() == GeckoProcessType_Default) failed: file /home/work/B2G-unagi/birch/modules/libpref/src/Preferences.cpp, line 1349
Assertion failure: ((bool)(__builtin_expect(!!(!NS_FAILED_impl(rv)), 1))) (Should be able to set a preference), at /home/work/B2G-unagi/birch/storage/src/VacuumManager.cpp:378
Fatal signal 11 (SIGSEGV) at 0x00000000 (code=1)
*** *** *** *** *** *** *** *** *** *** *** *** *** *** *** ***
Build fingerprint: 'toro/full_unagi/unagi:4.0.4.0.4.0.4/OPENMASTER/eng.dhylands.20130404.111326:eng/test-keys'
pid: 472, tid: 472  >>> /system/b2g/plugin-container <<<
signal 11 (SIGSEGV), code 1 (SEGV_MAPERR), fault addr 00000000
 r0 000000bd  r1 befb94d0  r2 0000007b  r3 00000000
 r4 42efb240  r5 42ec4250  r6 43e57328  r7 befb9928
 r8 4206aed6  r9 42efafdc  10 41fd585a  fp befb9c60
 ip 4002c108  sp befb9920  lr 40f644ed  pc 40f644f0  cpsr 20000030
 d0  61562f6372732f67  d1  616e614d6d757565
 d2  637269622f696772  d3  6761726f74732f2e
 d4  65636f7250746547  d5  2928657079547373
 d6  6b636547203d3d20  d7  737365636f72506f
 d8  406680537feb02ff  d9  0000000000000000
 d10 0000000000000000  d11 0000000000000000
 d12 0000000000000000  d13 0000000000000000
 d14 0000000000000000  d15 0000000000000000
 d16 6665445f65707954  d17 61662029746c7561
 d18 6966203a64656c69  d19 656d6f682f20656c
 d20 32422f6b726f772f  d21 2f6967616e752d47
 d22 6f6d2f6863726962  d23 696c2f73656c7564
 d24 0000005f00000031  d25 ff8dff4bff81ffbf
 d26 0000007e00000040  d27 00000072000000b3
 d28 0000000040396da4  d29 000000007e6688ae
 d30 00000000b48ca1b6  d31 00000000725e88b6
 scr 28000011

         #00  pc 00b584f0  /system/b2g/libxul.so
         #01  lr 40f644ed  /system/b2g/libxul.so

code around pc:
40f644d0 4b3f4a3e 483f4479 447b447a 90004478  >J?KyD?HzD{DxD..
40f644e0 70bdf44f 20079001 cae8f4a8 227b2300  O..p... .....#{"
40f644f0 f7ff601a 2000bb68 7707f507 ecbd46bd  .`..h.. ...w.F..
40f64500 e8bd8b02 f5078ff0 f8467606 46580d10  .........vF...XF
40f64510 fceaf7fe 68034631 479868db f6bf2800  ....1F.h.h.G.(..

code around lr:
40f644cc 493eda13 4b3f4a3e 483f4479 447b447a  ..>I>J?KyD?HzD{D
40f644dc 90004478 70bdf44f 20079001 cae8f4a8  xD..O..p... ....
40f644ec 227b2300 f7ff601a 2000bb68 7707f507  .#{".`..h.. ...w
40f644fc ecbd46bd e8bd8b02 f5078ff0 f8467606  .F...........vF.
40f6450c 46580d10 fceaf7fe 68034631 479868db  ..XF....1F.h.h.G

stack:
    befb98e0  4206e078  /system/b2g/libxul.so
    befb98e4  00000000  
    befb98e8  befb9908  [stack]
    befb98ec  4206aed6  /system/b2g/libxul.so
    befb98f0  42efafdc  
    befb98f4  41fd585a  /system/b2g/libxul.so
    befb98f8  befb9c60  [stack]
    befb98fc  6d0e558a  
    befb9900  00000545  
    befb9904  42efb240  
    befb9908  42ec4250  
    befb990c  43e57328  
    befb9910  befb9928  [stack]
    befb9914  40f644ed  /system/b2g/libxul.so
    befb9918  df0027ad  
    befb991c  00000000  
#00 befb9920  4206db89  /system/b2g/libxul.so
    befb9924  0000017a  
    befb9928  befb9930  [stack]
    befb992c  42ec4250  
    befb9930  426a45a8  /system/b2g/libxul.so
    befb9934  426a4578  /system/b2g/libxul.so
    befb9938  426a45a8  /system/b2g/libxul.so
    befb993c  4206e078  /system/b2g/libxul.so
    befb9940  00000000  
    befb9944  00000000  
    befb9948  42e23780  
    befb994c  426ead80  /system/b2g/libxul.so
    befb9950  42e4e150  
    befb9954  400c04f8  
    befb9958  00000002  
    befb995c  42961c21  /system/b2g/libnss3.so
    befb9960  42ec4250  
    befb9964  41415d65  /system/b2g/libxul.so

Comment 2

[Gabriele Svelto [:gsvelto]]

I hit a similar problem when dealing with bug 820438 and my solution (attachment 708649 [details] [diff] [review]) was to effectively by-pass setting the preference in the child process. Everytime a child would run it would read the preference (which it's allowed to do) and then store it's updated value in a static variable instead of setting it via Preferences::Set*().

I think this might also work here so you'd read 'storage.vacuum.last.index' once the first time VacuumManager::Observe() is called, store its value in a static variable and then update and use that instead of the preference the next time the observer runs.

Comment 3

[Marco Bonardo [:mak]]

this component should just not run in a child process

Comment 4

[Gregor Wagner [:gwagner]]

Same here:

E/GeckoConsole( 2930): [JavaScript Error: "[Exception... "Component returned failure code: 0x80004005 (NS_ERROR_FAILURE) [nsIProperties.get]"  nsresult: "0x80004005 (NS_ERROR_FAILURE)"  location: "JS frame :: resource://gre/modules/FormHistory.jsm :: <TOP_LEVEL> :: line 376"  data: no]" {file: "resource://gre/modules/FormHistory.jsm" line: 376}]
I/Gecko   ( 2930): [Child 2930] WARNING: NS_ENSURE_TRUE(XRE_GetProcessType() == GeckoProcessType_Default) failed: file /Volumes/2mac/gaia/src/modules/libpref/src/Preferences.cpp, line 1356
F/MOZ_Assert( 2930): Assertion failure: ((bool)(__builtin_expect(!!(!NS_FAILED_impl(rv)), 1))) (Should be able to set a preference), at /Volumes/2mac/gaia/src/storage/src/VacuumManager.cpp:378


After talking to bent we shouldn't create a VacumManager in the child.

Comment 5

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Yeah, mak is right, as usual :)

Comment 6

[Gregor Wagner [:gwagner]]

Attachment: patch

Comment 7

[Gregor Wagner [:gwagner]]

Comment on attachment 796943 [details] [diff] [review]
patch

Try is green: https://tbpl.mozilla.org/?tree=Try&rev=27e24862f69b
But I don't know how many non-main process tests we have for it.

Comment 8

[Ben Turner (not reading bugmail, use the needinfo flag!)]

Comment on attachment 796943 [details] [diff] [review]
patch

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

Does this lead to any scary warnings or anything during startup?

::: storage/src/VacuumManager.cpp
@@ +316,5 @@
>  
>  VacuumManager *
>  VacuumManager::getSingleton()
>  {
> +  //Don't allocate it in the child Process.

This should probably say something more like "Child processes cannot access SQLite databases directly and therefore this service does not need to be created."

Comment 9

[Gregor Wagner [:gwagner]]

https://hg.mozilla.org/integration/b2g-inbound/rev/8c5869c1f670

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/8c5869c1f670

Comment 11

[Tom S. (please needinfo tschuster)]

Sadly the ugly string prevailed, please fix.