465974 - Crash during start of firefox with weave installed

Status: RESOLVED FIXED

(Cloud Services :: General, defect)

Description

[Honza Bambas (:mayhemer)]

For reference this is backtrace:

#0	0x31555590 in NSSRWLock_LockRead_Util at nssrwlk.c:177
#1	0x30de3ead in SECMOD_GetReadLock at pk11list.c:71
#2	0x30e06614 in PK11_GetAllTokens at pk11slot.c:1778
#3	0x30e0685f in PK11_GetBestSlotMultiple at pk11slot.c:1868
#4	0x30e06aa7 in PK11_GetBestSlot at pk11slot.c:1929
#5	0x30e06e67 in PK11_GenerateRandom at pk11slot.c:2075
#6	0x30ad6cd7 in WeaveCrypto::GenerateRandomIV
#7	0x0187a6a4 in NS_InvokeByIndex_P at xptcinvoke_unixish_x86.cpp:179
#8	0x0b34eb8c in XPCWrappedNative::CallMethod at xpcwrappednative.cpp:2422
#9	0x0b356a2e in XPC_WN_CallMethod at xpcwrappednativejsops.cpp:1477
#10	0x010a7849 in js_Invoke at jsinterp.cpp:1313
#11	0x0107f600 in js_Interpret at jsinterp.cpp:5144
#12	0x010a78da in js_Invoke at jsinterp.cpp:1331
#13	0x0b3495ff in nsXPCWrappedJSClass::CallMethod at xpcwrappedjsclass.cpp:1549
#14	0x0b341b6f in nsXPCWrappedJS::CallMethod at xpcwrappedjs.cpp:563
#15	0x0187a95e in PrepareAndDispatch at xptcstubs_unixish_x86.cpp:93
#16	0x0187a9bc in nsXPTCStubBase::Stub3 at xptcstubsdef.inc:1
#17	0x11dc2178 in nsEventListenerManager::HandleEventSubType at nsEventListenerManager.cpp:1091
#18	0x11dc3ddb in nsEventListenerManager::HandleEvent at nsEventListenerManager.cpp:1196
#19	0x11ded8bc in nsEventTargetChainItem::HandleEvent at nsEventDispatcher.cpp:236
#20	0x11dedaf5 in nsEventTargetChainItem::HandleEventTargetChain at nsEventDispatcher.cpp:300
#21	0x11dee36c in nsEventDispatcher::Dispatch at nsEventDispatcher.cpp:514
#22	0x11aea385 in DocumentViewerImpl::LoadComplete at nsDocumentViewer.cpp:989
#23	0x332a6b67 in nsDocShell::EndPageLoad at nsDocShell.cpp:5175
#24	0x332af428 in nsWebShell::EndPageLoad at nsWebShell.cpp:1013
#25	0x33296143 in nsDocShell::OnStateChange at nsDocShell.cpp:5071
#26	0x332bf153 in nsDocLoader::FireOnStateChange at nsDocLoader.cpp:1235
#27	0x332bf4aa in nsDocLoader::doStopDocumentLoad at nsDocLoader.cpp:858
#28	0x332bf69f in nsDocLoader::DocLoaderIsEmpty at nsDocLoader.cpp:763
#29	0x3331489c in nsDocLoader::ChildDoneWithOnload at nsDocLoader.h:205
#30	0x332bf6cd in nsDocLoader::DocLoaderIsEmpty at nsDocLoader.cpp:766
#31	0x332c01ae in nsDocLoader::OnStopRequest at nsDocLoader.cpp:679
#32	0x0c273cad in nsLoadGroup::RemoveRequest at nsLoadGroup.cpp:688
#33	0x11d3b550 in nsDocument::DoUnblockOnload at nsDocument.cpp:6988
#34	0x11d47a0c in nsDocument::UnblockOnload at nsDocument.cpp:6935
#35	0x11d425b8 in nsDocument::DispatchContentLoadedEvents at nsDocument.cpp:3917
#36	0x1232db98 in nsRunnableMethod<nsDocument>::Run at nsThreadUtils.h:264
#37	0x01867d93 in nsThread::ProcessNextEvent at nsThread.cpp:510
#38	0x0180ad3b in NS_ProcessPendingEvents_P at nsThreadUtils.cpp:180
#39	0x0dc4d209 in nsBaseAppShell::NativeEventCallback at nsBaseAppShell.cpp:121
#40	0x0dc0c7bb in nsAppShell::ProcessGeckoEvents at nsAppShell.mm:373
#41	0x9082bf06 in CFRunLoopRunSpecific
#42	0x9082ba42 in CFRunLoopRunInMode
#43	0x92df2878 in RunCurrentEventLoopInMode
#44	0x92df1eb9 in ReceiveNextEventCommon
#45	0x92df1dd9 in BlockUntilNextEventMatchingListInMode
#46	0x93297f45 in _DPSNextEvent
#47	0x93297b37 in -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:]
#48	0x932918c4 in -[NSApplication run]
#49	0x0dc0b087 in nsAppShell::Run at nsAppShell.mm:692
#50	0x0f76761f in nsAppStartup::Run at nsAppStartup.cpp:192
#51	0x0021173c in XRE_main at nsAppRunner.cpp:3264
#52	0x000027a4 in main at nsBrowserApp.cpp:156

I will try to narrow the regression range.

Comment 1

[Honza Bambas (:mayhemer)]

JS stack is:

0 [native frame]
1 Crypto_checkModule() ["file:///Users/starapica/Library/Application%20Support/Firefox/Profiles/k5efr8ce.default/extensions/%7B340c2bbc-ce74-4362-90b5-7c26312808ef%7D/modules/crypto.js":129]
    ok = false
    this = [object Object]
2 Sync() ["chrome://weave/content/sync.js":57]
    username = undefined
    this = [object Object]
3 anonymous(e = [object Event @ 0x35dc1390 (native @ 0x36b3ce00)]) ["chrome://weave/content/sync.js":496]
    this = [object ChromeWindow @ 0x35dbbdb0 (native @ 0x35dbaf90)]

Comment 4

[Honza Bambas (:mayhemer)]

Cause is bug 462806. It seems Weave is using but not initializing PSM. Enough is to get service of "@mozilla.org/psm;1" with nsISupports interface AFAIK. This is definitely bug in Weave.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Uh... you're not using PSM at all.  You're using NSS directly, without initializing it.  "Don't Do That".

Comment 6

[Boris Zbarsky [:bzbarsky]]

And seriously, why _are_ you using NSS directly?  We have nsIRandomGenerator for what you want here.

Comment 7

[Nelson Bolyard (seldom reads bugmail)]

Boris, in the stack in comment 0, I see:

#6    0x30ad6cd7 in WeaveCrypto::GenerateRandomIV

The letters IV there denote "Initialization Vector", a thing used with 
certain forms of encryption.  From that, I infer that weave is trying
to do some encryption (or decryption), and it probably cares about doing
so in a way that meets certain cryptographic and/or business criteria
(such as, FIPS validate, for example), for which NSS qualifies and 
nsIRandomGenerator may or may not.  So I don't begrudge weave the use 
of NSS for good crypto (obviously :).  

I suppose that a question exists about whether weave should interface to
NSS directly, or through PSM.  Doing so through PSM solves a number of 
issues pertaining to initialization of NSS, I believe.

Comment 8

[Boris Zbarsky [:bzbarsky]]

Nelson, nsIRandomGenerator is just a PSM front-end for PK11_GenerateRandom.  I guess weave is doing a lot of crypto stuff, and if PSM doesn't expose APIs for all of it, then they can't use it for everything they want.  And I guess they're linking to NSS directly, of course.

I don't mind weave using NSS if it wants to, but if it doesn't do it through PSM, and does so in the same process as PSM, then it might step on PSM's toes, or vice versa.  That's up to them, of course.

Comment 9

[Honza Bambas (:mayhemer)]

As per bug 433949 they link with NSS directly. There is a lot of functionality that PSM doesn't expose and is needed by Weave. AllPeers did the same and was successful in it but it is a looong time ago in ages of 1.8.1, thinks changed from that time.

Comment 11

[Justin Dolske [:Dolske]]

Yeah, the Weave extension is doing lots of crypto stuff directly with NSS. The RNG stuff just happens to be the first thing that it's doing when this crash happens.

See: http://hg.mozilla.org/labs/weave/file/6f10cf33cf7f/src/

Comment 12

[Shawn Wilsher :sdwilsh]

In bug 466237 comment 0 I posted a possible solution that allows weave to safely use NSS APIs.

Comment 13

[Justin Dolske [:Dolske]]

Attachment: Patch v.1

Maybe like this? Untested, as the Weave unit tests don't seem to be working for me (even with just a clean Weave tree).

Comment 14

[Dan Mills [:thunder]]

Attachment: Patch v.2

Patch as committed.

Comment 15

[Dan Mills [:thunder]]

I'll spin a weave release asap.

Comment 16

[Kai Engert (:KaiE:)]

You should check that init of PSM succeeded. Because if it fails, then you'll crash, too.

If init failed, you should disable all functionality that depends on NSS.

Please also read bug 462806 comment 27. In that comment I mention events used to control the shutdown/init state. I explained those events recently in bug 463256 comment 33.

Comment 17

[chris hofmann]

is this ready to close?

Comment 18

[Dan Mills [:thunder]]

This should be ready to close.  WeaveCrypto initializes NSS, and that has been released for a while.