580790 - Connection::initialize can access preferences off of the main thread

Status: RESOLVED FIXED

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

Description

[Jesse Ruderman]

Hit these assertions while fuzzing, using a Mac32 Tinderbox build, built from http://hg.mozilla.org/mozilla-central/rev/1ac07fe5f6c9.  Both happened in the same session, but not together.  I wasn't able to reproduce.

Related to bug 380315?  Is this a bug in Storage, Prefs, or pldhash?

###!!! ASSERTION: RECURSION_LEVEL(table_) > 0: 'RECURSION_LEVEL(table) > 0', file pldhash.c, line 707
PL_DHashTableOperate [pldhash.c:707]
pref_HashTableLookup [modules/libpref/src/prefapi.cpp:676]
PREF_GetIntPref [modules/libpref/src/prefapi.cpp:459]
nsPrefBranch::GetIntPref [modules/libpref/src/nsPrefBranch.cpp:256]
nsPrefService::GetIntPref [modules/libpref/src/nsPrefService.h:60]
mozilla::storage::Connection::initialize [storage/src/mozStorageConnection.cpp:446]
mozilla::storage::Service::OpenDatabase [storage/src/mozStorageService.cpp:318]
nsUrlClassifierDBServiceWorker::OpenDb [toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp:3388]
nsUrlClassifierDBServiceWorker::BeginUpdate [toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp:2921]
NS_InvokeByIndex_P [xpcom/reflect/xptcall/src/md/unix/xptcinvoke_unixish_x86.cpp:179]
nsProxyObjectCallInfo::Run [xpcom/proxy/src/nsProxyEvent.cpp:181]
nsThread::ProcessNextEvent [xpcom/threads/nsThread.cpp:547]
NS_ProcessNextEvent_P [nsThreadUtils.cpp:250]
nsThread::ThreadFunc [xpcom/threads/nsThread.cpp:262]
_pt_root [nsprpub/pr/src/pthreads/ptthread.c:231]
libSystem.B.dylib + 0x2e81d
libSystem.B.dylib + 0x2e6a2

###!!! ASSERTION: op == PL_DHASH_LOOKUP || RECURSION_LEVEL(table) == 0: 'op == PL_DHASH_LOOKUP || RECURSION_LEVEL(table) == 0', file pldhash.c, line 612
PL_DHashTableOperate [pldhash.c:613]
pref_HashPref [modules/libpref/src/prefapi.cpp:689]
PREF_SetIntPref [modules/libpref/src/prefapi.cpp:303]
nsPrefBranch::SetIntPref [modules/libpref/src/nsPrefBranch.cpp:269]
nsPrefService::SetIntPref [modules/libpref/src/nsPrefService.h:60]
NS_InvokeByIndex_P [xpcom/reflect/xptcall/src/md/unix/xptcinvoke_unixish_x86.cpp:179]
CallMethodHelper::Invoke [js/src/xpconnect/src/xpcwrappednative.cpp:3061]
CallMethodHelper::Call [js/src/xpconnect/src/xpcwrappednative.cpp:2340]
XPCWrappedNative::CallMethod [js/src/xpconnect/src/xpcwrappednative.cpp:2304]
XPC_WN_CallMethod [js/src/xpconnect/src/xpcwrappednativejsops.cpp:1796]
js::callJSNative [js/src/jscntxtinlines.h:339]
Invoke<JSBool (*)(JSContext*, JSObject*, uintN, jsval*, jsval*)> [js/src/jsinterp.cpp:591]
js_Invoke [js/src/jsinterp.cpp:693]
js_Interpret [js/src/jsops.cpp:2155]
Invoke<JSBool (*)(JSContext*, JSObject*, uintN, jsval*, jsval*)> [js/src/jsinterp.cpp:602]
js_Invoke [js/src/jsinterp.cpp:693]
nsXPCWrappedJSClass::CallMethod [js/src/xpconnect/src/xpcwrappedjsclass.cpp:1689]
nsXPCWrappedJS::CallMethod [js/src/xpconnect/src/xpcwrappedjs.cpp:570]
PrepareAndDispatch [xpcom/reflect/xptcall/src/md/unix/xptcstubs_unixish_x86.cpp:93]
nsXPTCStubBase::Stub3 [xptcstubsdef.inc:1]
NS_InvokeByIndex_P [xpcom/reflect/xptcall/src/md/unix/xptcinvoke_unixish_x86.cpp:179]
CallMethodHelper::Invoke [js/src/xpconnect/src/xpcwrappednative.cpp:3061]
CallMethodHelper::Call [js/src/xpconnect/src/xpcwrappednative.cpp:2340]
XPCWrappedNative::CallMethod [js/src/xpconnect/src/xpcwrappednative.cpp:2304]
XPC_WN_CallMethod [js/src/xpconnect/src/xpcwrappednativejsops.cpp:1796]
js::callJSNative [js/src/jscntxtinlines.h:339]
Invoke<JSBool (*)(JSContext*, JSObject*, uintN, jsval*, jsval*)> [js/src/jsinterp.cpp:591]
js_Invoke [js/src/jsinterp.cpp:693]
js_Interpret [js/src/jsops.cpp:2155]
Invoke<JSBool (*)(JSContext*, JSObject*, uintN, jsval*, jsval*)> [js/src/jsinterp.cpp:602]
js_Invoke [js/src/jsinterp.cpp:693]
js_InternalInvoke [js/src/jsinterp.cpp:739]
js_InternalGetOrSet [js/src/jsinterp.cpp:767]
JSScopeProperty::get [js/src/jsscopeinlines.h:274]
js_NativeGet [js/src/jsobj.cpp:4752]
js_GetPropertyHelper [js/src/jsobj.cpp:4922]
js_GetProperty [js/src/jsobj.cpp:4933]
JSObject::getProperty [js/src/jsobj.h:660]
JS_GetPropertyById [js/src/jsapi.cpp:3655]
JS_GetProperty [js/src/jsapi.cpp:3668]
mozJSComponentLoader::ImportInto [js/src/xpconnect/loader/mozJSComponentLoader.cpp:1779]
mozJSComponentLoader::Import [js/src/xpconnect/loader/mozJSComponentLoader.cpp:1624]
nsXPCComponents_Utils::Import [js/src/xpconnect/src/xpccomponents.cpp:3754]
NS_InvokeByIndex_P [xpcom/reflect/xptcall/src/md/unix/xptcinvoke_unixish_x86.cpp:179]
CallMethodHelper::Invoke [js/src/xpconnect/src/xpcwrappednative.cpp:3061]
CallMethodHelper::Call [js/src/xpconnect/src/xpcwrappednative.cpp:2340]
XPCWrappedNative::CallMethod [js/src/xpconnect/src/xpcwrappednative.cpp:2304]
XPC_WN_CallMethod [js/src/xpconnect/src/xpcwrappednativejsops.cpp:1796]
js::callJSNative [js/src/jscntxtinlines.h:339]
Invoke<JSBool (*)(JSContext*, JSObject*, uintN, jsval*, jsval*)> [js/src/jsinterp.cpp:591]
js_Invoke [js/src/jsinterp.cpp:693]
js_Interpret [js/src/jsops.cpp:2155]
Invoke<JSBool (*)(JSContext*, JSObject*, uintN, jsval*, jsval*)> [js/src/jsinterp.cpp:602]
js_Invoke [js/src/jsinterp.cpp:693]
js_InternalInvoke [js/src/jsinterp.cpp:739]
JS_CallFunctionValue [js/src/jsapi.cpp:4850]
nsJSContext::CallEventHandler [dom/base/nsJSEnvironment.cpp:2204]
nsGlobalWindow::RunTimeout [dom/base/nsGlobalWindow.cpp:8502]
nsGlobalWindow::TimerCallback [dom/base/nsGlobalWindow.cpp:8849]
nsTimerImpl::Fire [xpcom/threads/nsTimerImpl.cpp:428]
nsTimerEvent::Run [xpcom/threads/nsTimerImpl.cpp:521]
nsThread::ProcessNextEvent [xpcom/threads/nsThread.cpp:547]
NS_ProcessPendingEvents_P [nsThreadUtils.cpp:200]
nsBaseAppShell::NativeEventCallback [widget/src/xpwidgets/nsBaseAppShell.cpp:127]
nsAppShell::ProcessGeckoEvents [widget/src/cocoa/nsAppShell.mm:395]
CoreFoundation + 0x3f0fb
CoreFoundation + 0x3cbbf
CoreFoundation + 0x3c094
CoreFoundation + 0x3bec1
HIToolbox + 0x34f9c
HIToolbox + 0x34d51
HIToolbox + 0x34bd6
AppKit + 0x48a89
-AppKit + 0x482ca
-AppKit + 0xa55b
nsAppShell::Run [widget/src/cocoa/nsAppShell.mm:747]
nsAppStartup::Run [toolkit/components/startup/src/nsAppStartup.cpp:191]
XRE_main [toolkit/xre/nsAppRunner.cpp:3630]
main [browser/app/nsBrowserApp.cpp:158]
firefox-bin + 0x1486

Comment 1

[Boris Zbarsky [:bzbarsky]]

Sounds like a storage bug to me; neither prefs nor pldhash claimed to be threadsafe when I last checked.

Comment 2

[Benjamin Smedberg]

Sure looks like it, unless calling mozilla::storage::Service::OpenDatabase is supposed to be a main-thread-only thing.

Comment 3

[Shawn Wilsher :sdwilsh]

It...is not.  I'm not sure if I just want to drop the preference, or get stuff off the main thread...

This is totally me.  Should this be blocking?

Comment 4

[Benjamin Smedberg]

Seems like it could cause pref crashes that won't show up obviously in crashreporter signatures, so yes.

Comment 5

[Benjamin Smedberg]

Sounds like we should add mainthread assertions to the prefservice too.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Would it make sense to not make it use threadsafe refcounting?  Just so people get those asserts?  Or is that more likely to just get us in trouble in release builds?

Comment 7

[Benjamin Smedberg]

I think that there are people who use XPCOM proxies with it still, so probably not for this cycle.

Comment 8

[Shawn Wilsher :sdwilsh]

And need bug 578939 (already a blocker) to do this sanely...

Comment 9

[Shawn Wilsher :sdwilsh]

Attachment: Prefs v1.0

This makes callers of the pref service go BOOM (NS_ERROR_UNEXPECTED) when they try to do anything with preferences off of the main thread.  This makes the storage tests that open a database connection off of the main thread fail.

Comment 10

[Shawn Wilsher :sdwilsh]

Attachment: Storage v1.0

And now the storage bits.  Pretty self explanatory.

Comment 11

[Shawn Wilsher :sdwilsh]

Attachment: Storage v1.0

It helps when one saves before qrefreshing...

Comment 12

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

Comment on attachment 460943 [details] [diff] [review]
Storage v1.0

So, the code seems reasonable, but this will potentially result in all databases initialized off the main thread not being initialized with the value of the preference.  Since I presume the preference was added in the first place because enough vocal people were concerned about fsync load on their system or database corruption on crashes, this might be concerning to those same vocal people.  Of note is that apparently fedora firefox builds set this value to 0 by default.

As such this is sorta a de facto API change, or at the very least something that I can shirk the blame to a super-reviewer on.  I'm setting sr? because I don't know who to direct it at, please pick somebody.

I do think correctness trumps the preference which is why I'm r+'ing.  If you know the set of currently off-main-thread ininitialized db's and that they are unlikely to cause problems, it might be good to mention that here or in a comment in the code.

Comment 13

[Shawn Wilsher :sdwilsh]

(In reply to comment #12)
> Comment on attachment 460943 [details] [diff] [review]
> Storage v1.0
> 
> So, the code seems reasonable, but this will potentially result in all
> databases initialized off the main thread not being initialized with the value
> of the preference.  Since I presume the preference was added in the first place
> because enough vocal people were concerned about fsync load on their system or
> database corruption on crashes, this might be concerning to those same vocal
> people.  Of note is that apparently fedora firefox builds set this value to 0
> by default.
We'll still use the default value, which isn't FULL at least.  Also, fedora should not be doing that if they are still calling it Firefox, so we may need to open a bug up for that...

> I do think correctness trumps the preference which is why I'm r+'ing.  If you
> know the set of currently off-main-thread ininitialized db's and that they are
> unlikely to cause problems, it might be good to mention that here or in a
> comment in the code.
Code comment is likely to be out of date.  Right now the only two databases would be indexedDB and urlclassifier.  Although, I don't see why someone would initialize a database on one thread (background thread) and then use it on another.  I would expect all of these to happen on a thread where possibly more fsyncs would matter.  The only thing this actually sucks for is people who set it to 2.

Comment 14

[Mike Shaver (:shaver emeritus)]

If we're going to ignore the pref, we should remove it everywhere.  If not, then we should honour it.  We don't need to support changes to this without a restart, so I think we should just load the pref's value into an int somewhere early on, and then the different threads can read that int.  Easy, fast, safe.

Comment 15

[Shawn Wilsher :sdwilsh]

(In reply to comment #14)
> If we're going to ignore the pref, we should remove it everywhere.  If not,
> then we should honour it.  We don't need to support changes to this without a
> restart, so I think we should just load the pref's value into an int somewhere
> early on, and then the different threads can read that int.  Easy, fast, safe.
And that's what this patch does, except when the storage service gets initialized off of the main thread.  We can't get the pref value there, so we dump an event to the main thread to get it for us.  I suppose we could proxy it, but in the rare case that we get initialized off of the main thread, I don't think this will kill us.

Comment 16

[Shawn Wilsher :sdwilsh]

(fwiw, I don't think, because of places, that the service will ever be initialized off of the main thread in the browser)

Comment 17

[Mike Shaver (:shaver emeritus)]

Ah, sorry, yes!  Can we warn if storage initialized off-thread, or perhaps even error out?  Fine for a follow-up, this is strictly improvement now.

Comment 18

[Mike Shaver (:shaver emeritus)]

Comment on attachment 460943 [details] [diff] [review]
Storage v1.0

sr=shaver, thanks.

Comment 19

[Shawn Wilsher :sdwilsh]

(In reply to comment #17)
> Ah, sorry, yes!  Can we warn if storage initialized off-thread, or perhaps even
> error out?  Fine for a follow-up, this is strictly improvement now.
Well, it is something we explicitly support (see dependent bug).  Up until now, it would not have mattered if we were initialized on or off of the main thread - stuff that would work on a background thread would have worked still.  The preference is the only thing that breaks.  Not sure if it is disallowing initializing the service off of the main thread (or even warning) as a result of this.  Database connections can still set PRAGMA synchronous on their own (which we do at least for places and cookies) if they really care what it gets set to.

Comment 20

[[e-mail]]

Comment on attachment 460929 [details] [diff] [review]
Prefs v1.0

Gah! Do we have to do this in release builds? Make it an assert, like it is in NS_IMPL_ADDREF: http://mxr.mozilla.org/mozilla-central/source/xpcom/glue/nsISupportsImpl.h#342

Also I'd rather see a macro for all this copy/pasted code, or at least fold the NS_IsMainThread() check and LogBackgroundThreadUse() into a single inline function.

r=dwitte with that.

Comment 21

[Shawn Wilsher :sdwilsh]

(In reply to comment #20)
> Gah! Do we have to do this in release builds? Make it an assert, like it is in
> NS_IMPL_ADDREF:
> http://mxr.mozilla.org/mozilla-central/source/xpcom/glue/nsISupportsImpl.h#342
The whole point was to do this in release builds because add-ons don't usually test in debug builds.

> Also I'd rather see a macro for all this copy/pasted code, or at least fold the
> NS_IsMainThread() check and LogBackgroundThreadUse() into a single inline
> function.
bsmedberg doesn't like things that return to be inside macros, which is why I didn't do that.

Comment 22

[[e-mail]]

(In reply to comment #21)
> The whole point was to do this in release builds because add-ons don't usually
> test in debug builds.

What about every other service that's not threadsafe that could be used by addons? Granted, prefservice is a bigger target, but adding release checks to each interface method and accessor on a bunch of other services is a non-starter.

Is there a way to have the servicemgr throw if you access a non-threadsafe service off main thread? I don't *think* there is. We could potentially add something to nsIClassInfo.flags for that purpose. Or alter the signature of _InstanceClass##Constructor (http://mxr.mozilla.org/mozilla-central/source/xpcom/components/ModuleUtils.h#105) to return a 'threadsafe' outparam, which the servicemgr could stash in its flags.

Not sure if any of this is worth it... bsmedberg?

> > Also I'd rather see a macro for all this copy/pasted code, or at least fold the
> > NS_IsMainThread() check and LogBackgroundThreadUse() into a single inline
> > function.
> bsmedberg doesn't like things that return to be inside macros, which is why I
> didn't do that.

Fair enough -- roll them into a CheckMainThread() function?

Comment 23

[Shawn Wilsher :sdwilsh]

(In reply to comment #22)
> What about every other service that's not threadsafe that could be used by
> addons? Granted, prefservice is a bigger target, but adding release checks to
> each interface method and accessor on a bunch of other services is a
> non-starter.
We added them to Places in Firefox 3.5.  the NS_IsMainThread checks are supposed to be fast now, and PGO should make it not main thread branch be the uncommon case.

> Fair enough -- roll them into a CheckMainThread() function?
Isn't this really what LogBackgroundThreadUse is essentially?  I mean, we'd go from:
  if (!NS_IsMainThread()) {
    nsPrefService::LogBackgroundThreadUse();
    return NS_ERROR_UNEXPECTED;
  }
to:
  if (!nsPrefService::CheckAndLogBackgroundThreadUse()) {
    return NS_ERROR_UNEXPECTED;
  }
which I can do, but seems like marginal gain.

Comment 24

[[e-mail]]

(In reply to comment #22)
> Is there a way to have the servicemgr throw if you access a non-threadsafe
> service off main thread? I don't *think* there is. We could potentially add
> something to nsIClassInfo.flags

Nevermind, we have one! http://mxr.mozilla.org/mozilla-central/source/xpcom/components/nsIClassInfo.idl#113

We could check it here-ish: http://mxr.mozilla.org/mozilla-central/source/xpcom/components/nsComponentManager.cpp#1391

I'd like to get bsmedberg's input on this. I'm OK with the checks as you have them if he is; I just don't like specialcasing the prefservice and paying the cost on every call.

(In reply to comment #23)
> We added them to Places in Firefox 3.5.  the NS_IsMainThread checks are
> supposed to be fast now, and PGO should make it not main thread branch be the
> uncommon case.

Please wrap it in NS_LIKELY.

> which I can do, but seems like marginal gain.

Please do. If you're copy-pasting code, make it as short as possible!

Comment 25

[Benjamin Smedberg]

I think we should pay the cost on every call: it's a trivially small cost on all the platforms we care about.

The component-manager flag doesn't help much now, and my post-FF4 thoughts will make it irrelevant, so let's not worry about it now.

Comment 26

[[e-mail]]

Ok, r=me then!

Comment 27

[Shawn Wilsher :sdwilsh]

(In reply to comment #24)
> Please wrap it in NS_LIKELY.
Presume you mean NS_UNLIKELY.  New patch RSN, please take a look.

Comment 28

[Shawn Wilsher :sdwilsh]

Attachment: Prefs v1.1

Comment 29

[Shawn Wilsher :sdwilsh]

The prefs changed landed, but I had a compiler issue with the storage part:
http://hg.mozilla.org/mozilla-central/rev/1302220623fd

Comment 30

[Shawn Wilsher :sdwilsh]

Except that I just backed that out too since I realized the pref changes by themselves will make Bd go orange.  *sigh*

Comment 31

[Shawn Wilsher :sdwilsh]

Based on try server results, bug 585706 and bug 593046 are the only two things blocking this from landing.

Comment 32

[Shawn Wilsher :sdwilsh]

Changing this to beta N since it does change the behavior of the prefs API (admittedly to throw on something people shouldn't have been doing anyway, but still a change).

Comment 33

[Benjamin Smedberg]

Not a blocker any more because we added thread checks to prefservice which prevents the crashes/corruption. We should still fix this because you'll get asserts (and you won't get the prefs you expect anyway).

Comment 34

[Shawn Wilsher :sdwilsh]

Preferences code spun off to bug 619487.

Comment 35

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

sdwilsh, can we land the storage patch since the preferences logic has been spun off onto its own bug (and fixed)?  It looks like the whiteboard annotation about needing bug 585706 was for the preferences part of this bug (when it was more explodey).

Comment 36

[Shawn Wilsher :sdwilsh]

(In reply to comment #35)
> sdwilsh, can we land the storage patch since the preferences logic has been
> spun off onto its own bug (and fixed)?  It looks like the whiteboard annotation
> about needing bug 585706 was for the preferences part of this bug (when it was
> more explodey).
probably, although it may have bitrotted.  I need to see if tests pass now.

Comment 37

[John J. Barton]

This patch causes messages in Error Console:

Invalid use of the preferences on a background thread!

These message have no value: there is no call stack, no filename, no line number, and you have to search through the Firefox source code to realize that it is caused by the platform code. 

Please remove this message or change it to be useful.

Comment 38

[[e-mail]]

Try setting a breakpoint in a debug build?

Comment 39

[Ben Bucksch (:BenB)]

> Try setting a breakpoint in a debug build?

You're asking JS developers to do that??

Are JS modules affected? I.e. can we use prefs in JS modules at least? I am not sure where they run, but they sure need prefs.

Please make preferences thread-safe instead of asking all apps and extensions to adapt.

Comment 40

[Shawn Wilsher :sdwilsh]

The patch that everyone is complaining about is actually bug 619487.  Please take the discussion there and stop polluting this bug.

Comment 42

[Ben Bucksch (:BenB)]

FWIW, this bug is now causing a Warning with the following stack. It should be solved by the Storage 1.0 patch.

Breakpoint 1, nsPrefService::CheckAndLogBackgroundThreadUse ()
    at /home/laga/work/firefox/source/modules/libpref/src/nsPrefService.cpp:983
983       NS_WARNING("Cannot be used on a background thread!");
(gdb) bt
#0  nsPrefService::CheckAndLogBackgroundThreadUse ()
    at /home/laga/work/firefox/source/modules/libpref/src/nsPrefService.cpp:983
#1  0x00007ffff4cd5ce5 in nsPrefBranch::GetIntPref (this=0x7fffebbee400,
    aPrefName=0x7ffff6cf4f83 "toolkit.storage.synchronous", _retval=0x7fffd23fe64c)
    at /home/laga/work/firefox/source/modules/libpref/src/nsPrefBranch.cpp:235
#2  0x00007ffff4cde999 in nsPrefService::GetIntPref (this=0x7fffe6861920,
    aPrefName=0x7ffff6cf4f83 "toolkit.storage.synchronous", _retval=0x7fffd23fe64c)
    at /home/laga/work/firefox/source/modules/libpref/src/nsPrefService.h:62
#3  0x00007ffff5d9081f in mozilla::storage::Connection::initialize (this=0x7fffd26ede40,
    aDatabaseFile=0x7fffd26eda80, aVFSName=0x0)
    at /home/laga/work/firefox/source/storage/src/mozStorageConnection.cpp:474
#4  0x00007ffff5d8df2c in mozilla::storage::Service::OpenDatabase (this=0x7fffe3283e20,
    aDatabaseFile=0x7fffd26eda80, _connection=0x7fffd23fe800)
    at /home/laga/work/firefox/source/storage/src/mozStorageService.cpp:454
#5  0x00007ffff5c246c6 in nsUrlClassifierDBServiceWorker::OpenDb (this=0x7fffd354a000)
    at /home/laga/work/firefox/source/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp:3391
#6  0x00007ffff5c22b02 in nsUrlClassifierDBServiceWorker::BeginUpdate (this=0x7fffd354a000,
    observer=0x7fffd26e17c0, tables=..., clientKey=...)
    at /home/laga/work/firefox/source/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp:2921
#7  0x00007ffff62660a6 in NS_InvokeByIndex_P (that=0x7fffd354a000, methodIndex=6,
    paramCount=3, params=0x7fffd2446470)
    at /home/laga/work/firefox/source/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_unix.cpp:208
#8  0x00007ffff625152b in nsProxyObjectCallInfo::Run (this=0x7fffd24464c0)
    at /home/laga/work/firefox/source/xpcom/proxy/src/nsProxyEvent.cpp:181
#9  0x00007ffff62466f2 in nsThread::ProcessNextEvent (this=0x7fffd26ed9c0, mayWait=1,
    result=0x7fffd23fed7c) at /home/laga/work/firefox/source/xpcom/threads/nsThread.cpp:633
#10 0x00007ffff61d0178 in NS_ProcessNextEvent_P (thread=0x7fffd26ed9c0, mayWait=1)
    at nsThreadUtils.cpp:250
#11 0x00007ffff62456bd in nsThread::ThreadFunc (arg=0x7fffd26ed9c0)
    at /home/laga/work/firefox/source/xpcom/threads/nsThread.cpp:278
#12 0x00007ffff397998f in _pt_root (arg=0x7fffd3aef9b0)
    at /home/laga/work/firefox/source/nsprpub/pr/src/pthreads/ptthread.c:187
#13 0x00007ffff7bc8cb0 in start_thread () from /lib/libpthread.so.0
#14 0x00007fffefe2b9dd in clone () from /lib/libc.so.6
#15 0x0000000000000000 in ?? ()




#0  nsPrefService::CheckAndLogBackgroundThreadUse ()
    at /home/laga/work/firefox/source/modules/libpref/src/nsPrefService.cpp:983
#1  0x00007ffff4cd5ce5 in nsPrefBranch::GetIntPref (this=0x7fffebbee400,
    aPrefName=0x7ffff6cf4f83 "toolkit.storage.synchronous", _retval=0x7fffd23fe62c)
    at /home/laga/work/firefox/source/modules/libpref/src/nsPrefBranch.cpp:235
#2  0x00007ffff4cde999 in nsPrefService::GetIntPref (this=0x7fffe6861920,
    aPrefName=0x7ffff6cf4f83 "toolkit.storage.synchronous", _retval=0x7fffd23fe62c)
    at /home/laga/work/firefox/source/modules/libpref/src/nsPrefService.h:62
#3  0x00007ffff5d9081f in mozilla::storage::Connection::initialize (this=0x7fffd26ede40,
    aDatabaseFile=0x7fffd26eda80, aVFSName=0x0)
    at /home/laga/work/firefox/source/storage/src/mozStorageConnection.cpp:474
#4  0x00007ffff5d8df2c in mozilla::storage::Service::OpenDatabase (this=0x7fffe3283e20,
    aDatabaseFile=0x7fffd26eda80, _connection=0x7fffd23fe7e0)
    at /home/laga/work/firefox/source/storage/src/mozStorageService.cpp:454
#5  0x00007ffff5c246c6 in nsUrlClassifierDBServiceWorker::OpenDb (this=0x7fffd354a000)
    at /home/laga/work/firefox/source/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp:3391
#6  0x00007ffff5c23bb6 in nsUrlClassifierDBServiceWorker::ApplyUpdate (this=0x7fffd354a000)
    at /home/laga/work/firefox/source/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp:3210
#7  0x00007ffff5c23cf5 in nsUrlClassifierDBServiceWorker::FinishUpdate (this=0x7fffd354a000)
    at /home/laga/work/firefox/source/toolkit/components/url-classifier/src/nsUrlClassifierDBService.cpp:3234
#8  0x00007ffff62660a6 in NS_InvokeByIndex_P (that=0x7fffd354a000, methodIndex=10,
    paramCount=0, params=0x0)
    at /home/laga/work/firefox/source/xpcom/reflect/xptcall/src/md/unix/xptcinvoke_x86_64_unix.cpp:208
#9  0x00007ffff625152b in nsProxyObjectCallInfo::Run (this=0x7fffd24466f0)
    at /home/laga/work/firefox/source/xpcom/proxy/src/nsProxyEvent.cpp:181
#10 0x00007ffff62466f2 in nsThread::ProcessNextEvent (this=0x7fffd26ed9c0, mayWait=1,
    result=0x7fffd23fed7c) at /home/laga/work/firefox/source/xpcom/threads/nsThread.cpp:633
#11 0x00007ffff61d0178 in NS_ProcessNextEvent_P (thread=0x7fffd26ed9c0, mayWait=1)
    at nsThreadUtils.cpp:250
#12 0x00007ffff62456bd in nsThread::ThreadFunc (arg=0x7fffd26ed9c0)
    at /home/laga/work/firefox/source/xpcom/threads/nsThread.cpp:278
#13 0x00007ffff397998f in _pt_root (arg=0x7fffd3aef9b0)
    at /home/laga/work/firefox/source/nsprpub/pr/src/pthreads/ptthread.c:187
#14 0x00007ffff7bc8cb0 in start_thread () from /lib/libpthread.so.0
#15 0x00007fffefe2b9dd in clone () from /lib/libc.so.6
#16 0x0000000000000000 in ?? ()

Comment 43

[Shawn Wilsher :sdwilsh]

(In reply to comment #42)
> FWIW, this bug is now causing a Warning with the following stack. It should be
> solved by the Storage 1.0 patch.
That's exactly why this bug was filed...

Comment 44

[Shawn Wilsher :sdwilsh]

(In reply to comment #33)
> Not a blocker any more because we added thread checks to prefservice which
> prevents the crashes/corruption. We should still fix this because you'll get
> asserts (and you won't get the prefs you expect anyway).
Except that that didn't actually stick, and was spun off into bug 619487.  I still think we should softblock on this.

Comment 45

[Shawn Wilsher :sdwilsh]

Attachment: Storage v1.1

Unbitrotting attachment 460943 [details] [diff] [review].  This now passes tests locally.

Comment 46

[Shawn Wilsher :sdwilsh]

Pushed to try: http://tbpl.mozilla.org/?tree=MozillaTry&rev=9b053ab2d5dd

Comment 47

[Shawn Wilsher :sdwilsh]

http://hg.mozilla.org/mozilla-central/rev/16734797ece9