1194319 - startup crash in nsPermissionManager::InitDB, spiking in 41 beta

Status: RESOLVED FIXED

(Core :: Permission Manager, defect)

Description

[Robert Kaiser]

This bug was filed from the Socorro interface and is 
report bp-f3c7c6bd-1519-42e8-b1bf-3526e2150813.
=============================================================

Top Stack Frames:
0 	xul.dll 	nsPermissionManager::InitDB(bool) 	extensions/cookie/nsPermissionManager.cpp
1 	xul.dll 	nsPermissionManager::Init() 	extensions/cookie/nsPermissionManager.cpp
2 	xul.dll 	nsPermissionManager::GetXPCOMSingleton() 	extensions/cookie/nsPermissionManager.cpp
3 	xul.dll 	nsIPermissionManagerConstructor 	extensions/cookie/nsCookieModule.cpp
4 	xul.dll 	mozilla::GenericFactory::CreateInstance(nsISupports*, nsID const&, void**) 	xpcom/glue/GenericFactory.cpp
5 	xul.dll 	nsComponentManagerImpl::CreateInstanceByContractID(char const*, nsISupports*, nsID const&, void**) 	xpcom/components/nsComponentManager.cpp
6 	xul.dll 	nsComponentManagerImpl::GetServiceByContractID(char const*, nsID const&, void**) 	xpcom/components/nsComponentManager.cpp
7 	xul.dll 	nsTHashtable<nsBaseHashtableET<nsCStringHashKey, nsFactoryEntry*> >::s_HashKey(PLDHashTable*, void const*) 	xpcom/glue/nsTHashtable.h
8 	xul.dll 	nsContentBlocker::Init() 	extensions/permissions/nsContentBlocker.cpp
9 	xul.dll 	mozilla::GenericFactory::CreateInstance(nsISupports*, nsID const&, void**) 	xpcom/glue/GenericFactory.cpp
10 	xul.dll 	nsComponentManagerImpl::CreateInstanceByContractID(char const*, nsISupports*, nsID const&, void**) 	xpcom/components/nsComponentManager.cpp
[...]

This signature seems to have been around in low volume on other versions, but seems to spike in 41 beta 1. It's on startup, which makes it rather nasty, and consistently seems to have an address of 0x0, which smells like a null deref.

Comment 1

[Robert Kaiser]

[Tracking Requested - why for this release]:
High-ranking startup crash in 41.0b1

Comment 2

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Tracked as it is a top crash.

Comment 3

[(Away)]

mDBConn is null.

From blame on nearby lines, one possibility I see is:
Bug 1124126 - Retry database connection for the case of corrupted permissions.sqlite. r=bsmedberg 

    1.59    rv = storage->OpenDatabase(permissionsFile, getter_AddRefs(mDBConn));
    1.60 -  NS_ENSURE_SUCCESS(rv, rv);
    1.61 +  if (rv == NS_ERROR_FILE_CORRUPTED) {

What if rv was some other error code?

Another possibility may be:
Bug 967812 - Pref to make Permissions Manager memory-only. r=ehsan

Comment 4

[(Away)]

Needinfo sitting for several days (I tried pinging on IRC). This is a startup crash so we need to act quickly. Ehsan, maybe you can look at this sooner? I don't expect that this is related to the change you reviewed, but perhaps you could point me to someone who knows this code.

Comment 5

[(no longer active)]

The most likely theory that I can come up with is the DB initialization failing with something other than NS_ERROR_UNEXPECTED, which is entirely possible.

Comment 6

[(no longer active)]

Attachment: Correctly deal with all possible ways that initializing the permission manager DB connection can fail

Comment 7

[(no longer active)]

Comment on attachment 8649468 [details] [diff] [review]
Correctly deal with all possible ways that initializing the permission manager DB connection can fail

Approval Request Comment
[Feature/regressing bug #]: Probably bug 967812
[User impact if declined]: I'd like to see if this patch will fix the startup crash, so I'd like to take this into beta ASAP.
[Describe test coverage new/current, TreeHerder]: This component has some tests, but we don't know how to reproduce the crash, so I'm just guessing for now.
[Risks and why]: Very low risk.
[String/UUID change made/needed]: None.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/9cf478be459a

Comment 9

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8649468 [details] [diff] [review]
Correctly deal with all possible ways that initializing the permission manager DB connection can fail

Approved for uplift to Beta and Aurora as it's a top ranking crash.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/9cf478be459a

Comment 11

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/9fe71b0975bb

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/733f333f82c2

Comment 13

[(no longer active)]

Kairo, what's the next step here?  I want to make sure that this doesn't fall through the cracks, and someone would end up verifying my guess.  :-)

Comment 14

[(Away)]

There isn't enough volume to verify this on nightly or aurora, so we'll need to wait for 41b3 data to come in. Don't worry, this is definitely on my radar and I bet KaiRo's too :-)

Comment 15

[Robert Kaiser]

(In reply to Ehsan Akhgari (don't ask for review please) from comment #13)
> Kairo, what's the next step here?  I want to make sure that this doesn't
> fall through the cracks, and someone would end up verifying my guess.  :-)

We are closely watching beta crash stats all the time. If it continues to appear there, you can be sure we come back and nag here. If it doesn't, it may just disappear off our radar or we may remember to report back that it's successfully gone. That's at least the usual thing that happens with beta crash fixes.

Comment 16

[(no longer active)]

Great, so I'll wait to see if this keeps showing up on beta!  Fingers crossed.  :-)

Comment 17

[Robert Kaiser]

And FWIW, both the signature summary behind the link in the Crash Signature field of this bug, and my searches for crashes on 41.0b3 confirm that this signature is not seen any more in beta 3, so this fix seems to have worked. :)

Comment 18

[(no longer active)]

\m/