612072 - Crash [@ nsCookieService::EnsureReadDomain(nsCString const&) ]

Status: RESOLVED FIXED

(Core :: Networking: Cookies, defect)

Description

[Mitchell Field [:Mitch]]

http://crash-stats.mozilla.com/report/index/1a55a928-b026-4a18-b18e-59d802101113

Firefox trunk nightly, Windows 7 x64.

Comment 1

[timeless]

Signature	nsCookieService::EnsureReadDomain(nsCString const&)
UUID	1a55a928-b026-4a18-b18e-59d802101113
Time 	2010-11-13 20:26:06.186717
Uptime	18
Last Crash	24 seconds before submission
Install Age	7821 seconds (2.2 hours) since version was first installed.
Product	Firefox
Version	4.0b8pre
Build ID	20101113042433
Branch	2.0
OS	Windows NT
OS Version	6.1.7600
CPU	x86
CPU Info	GenuineIntel family 6 model 23 stepping 6
Crash Reason	EXCEPTION_ACCESS_VIOLATION_READ
Crash Address	0x34
User Comments	
App Notes 	AdapterVendorID: 1002, AdapterDeviceID: 9591
Processor Notes 	
EMCheckCompatibility	False
Related Bugs

Crashing Thread
Frame 	Module 	Signature [Expand] 	Source
0 	xul.dll 	nsCookieService::EnsureReadDomain 	netwerk/cookie/nsCookieService.cpp:1943
1 	xul.dll 	nsCookieService::GetCookieStringInternal 	netwerk/cookie/nsCookieService.cpp:2354
2 	xul.dll 	nsCookieService::GetCookieStringCommon 	netwerk/cookie/nsCookieService.cpp:1449
3 	xul.dll 	nsCookieService::GetCookieStringFromHttp 	netwerk/cookie/nsCookieService.cpp:1431
4 	xul.dll 	mozilla::net::HttpBaseChannel::AddCookiesToRequest 	netwerk/protocol/http/HttpBaseChannel.cpp:1260
5 	xul.dll 	nsHttpChannel::AsyncOpen 	netwerk/protocol/http/nsHttpChannel.cpp:3555

1937 nsCookieService::EnsureReadDomain(const nsCString &aBaseDomain)
1938 {
1939   NS_ASSERTION(!mDBState->dbConn || mDBState == mDefaultDBState,
1940     "not in default db state");
1941
1942   // Fast path 1: nothing to read, or we've already finished reading.
1943   if (NS_LIKELY(!mDBState->dbConn || !mDefaultDBState->pendingRead))
1944     return; 

One of the pointers on line 1943 is null

    4.87  nsCookieService::nsCookieService()
    4.88 - : mDBState(&mDefaultDBState)
    4.89 + : mDBState(NULL)

My money is on mDBState :)

I don't have a current build here so I can't easily calculate which field is 0x34 (it mostly depends on sizeof nsTHashtable<>)

Comment 2

[[e-mail]]

Hmm. It could be either, since mDBState is likely a pointer to mDefaultDBState. (The latter is also constructed as null by virtue of being an nsRefPtr.)

I'll pull the offset and figure out which it is. I doubt I can reproduce this (I haven't seen it in all my testing) so I'll probably have to inspect.

Comment 3

[[e-mail]]

0x34 is dbConn, so mDBState is NULL. Given that this happened well after startup, it's likely that mDBState was nonnull at some point, and somehow got reset.

Comment 4

[Shawn Wilsher :sdwilsh]

(In reply to comment #3)
> 0x34 is dbConn, so mDBState is NULL. Given that this happened well after
> startup, it's likely that mDBState was nonnull at some point, and somehow got
> reset.
private browsing?

Comment 5

[[e-mail]]

There are 108 crashes since these patches landed a few days ago; all Windows, all 0x34. I don't think PB is involved since Mitch said he didn't use it during that session.

Which leaves three possibilities:
1) CloseDBStates() is being called via profile-before-change. This is impossible, since the crash didn't occur during shutdown. (Unless we're calling it at some other point!)
2) 'this' is NULL. (The offset of mDefaultDBState happens to be 0x34 also.) This is impossible, since we'd crash dereferencing mDBState first, which is not at 0x34.
3) The crash is actually occurring on a different line, but the optimizer's messing with us. This is impossible, since we've already nullchecked mDefaultDBState->pendingRead, so we couldn't try to deref it later and discover it's NULL.

Comment 7

[[e-mail]]

bsmedberg suggested I take a look at the minidump directly. I'll do that tomorrow.

Comment 8

[[e-mail]]

I've looked at the minidump, and confirmed that mDBState is NULL with certainty.

Comment 9

[[e-mail]]

Attachment: patch

There's nothing really preventing stuff from happening after profile-before-change, so the simple thing is just to guard against it at all the API entrypoints. This didn't used to be a problem because we always had a DBState around, empty or not, whereas now we actually delete it.

Now, I have to go through the in-tree consumers to make sure they can deal with this throwing...

Comment 10

[Shawn Wilsher :sdwilsh]

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

r=sdwilsh

Comment 11

[[e-mail]]

I hit this crash during shutdown, which explains why mDBState is NULL.

JS stack:
> 0 anonymous() ["resource://gre/modules/LightweightThemeManager.jsm":238]
    self = [object Object]
    req = [object XMLHttpRequest]
    theme = [object Object]
    this = [object Object]
> 1 AMI_backgroundUpdateCheck() ["resource://gre/modules/AddonManager.jsm":312]
    scope = [object Object]
    notifyComplete = [function]
    pendingUpdates = 1
    this = [object Object]
> 2 AMP_backgroundUpdateCheck() ["resource://gre/modules/AddonManager.jsm":855]
    this = [object Object]
> 3 AMC_notify(aTimer = [xpconnect wrapped nsITimer]) ["resource://gre/components/addonManager.js":156]
    this = [object Object]
> 4 TM_notify(timer = [xpconnect wrapped nsITimer]) ["resource://gre/components/nsUpdateTimerManager.js":158]
    timerData = undefined
    timerID = undefined
    entries = [xpconnect wrapped nsISimpleEnumerator]
    catMan = [xpconnect wrapped nsICategoryManager]
    now = 1290425305
    lastUpdateTime = 1290338584
    prefLastUpdate = "app.update.lastUpdateTime.addon-background-update-timer"
    this = [object Object]

Native stack:
nsCookieService::EnsureReadDomain
nsCookieService::GetCookieStringInternal
nsCookieService::GetCookieStringCommon
nsCookieService::GetCookieStringFromHttp
mozilla::net::HttpBaseChannel::AddCookiesToRequest
nsHttpChannel::AsyncOpen
nsXMLHttpRequest::Send
nsIXMLHttpRequest_Send
js::CallJSNative
js::Interpret
js::RunScript
js::Invoke
js::ExternalInvoke
js::ExternalInvoke
JS_CallFunctionValue
nsXPCWrappedJSClass::CallMethod
nsXPCWrappedJS::CallMethod
PrepareAndDispatch
SharedStub
NS_InvokeByIndex_P
CallMethodHelper::Invoke
CallMethodHelper::Call
XPCWrappedNative::CallMethod
XPC_WN_CallMethod
js::CallJSNative
js::Interpret
js::RunScript
js::Invoke
js::ExternalInvoke
js::ExternalInvoke
JS_CallFunctionValue
nsXPCWrappedJSClass::CallMethod
nsXPCWrappedJS::CallMethod
PrepareAndDispatch
SharedStub
nsTimerImpl::Fire
nsTimerEvent::Run
nsThread::ProcessNextEvent
NS_ProcessNextEvent_P
nsThread::Shutdown
NS_InvokeByIndex_P
nsProxyObjectCallInfo::Run
nsThread::ProcessNextEvent
NS_ProcessNextEvent_P
nsThread::Shutdown
nsCacheService::Shutdown
nsCacheProfilePrefObserver::Observe
nsObserverList::NotifyObservers
nsObserverService::NotifyObservers
mozilla::ShutdownXPCOM
NS_ShutdownXPCOM_P
ScopedXPCOMStartup::~ScopedXPCOMStartup
XRE_main
NS_internal_main
wmain
wmainCRTStartup
BaseProcessStart

Comment 12

[Sheila Mooney]

I know it's targeted for betaN but is this going to get checked in for beta8?

Comment 13

[[e-mail]]

Definitely. I'll push it today, pending results from try.

Comment 14

[[e-mail]]

http://hg.mozilla.org/mozilla-central/rev/74b4a2f97e23

Comment 15

[chris hofmann]

In this query I see a few reports in builds from Nov 23 (20101123070903 )  and 24 ( 20101124080547 ) but volume is dramatically lower and no reports after that.

http://crash-stats.mozilla.com/report/list?signature=nsCookieService%3A%3AEnsureReadDomain%28nsCString%20const%26%29

maybe a lower volume old bug that is still around?

http://crash-stats.mozilla.com/report/index/089e7d49-5508-4879-a75c-c41fa2101125
http://crash-stats.mozilla.com/report/index/9953d171-4620-4fa9-8d02-b92b32101127
http://crash-stats.mozilla.com/report/index/398d8133-14a3-4a9b-be35-b39cf2101124

Comment 16

[[e-mail]]

Hmm. Nothing after 20101124, and it's been a while. So I think this is fixed.