797302 - crash in mozilla::safebrowsing::Classifier::Check

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, defect)

Description

[Scoobidiver (away)]

There's a small spike in crashes from 17.0a1/20120826. The regression range might be:
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=f077de66e52d&tochange=b3cce81fef1a
It's likely a regression from bug 673470 that landed a few builds sooner.

Signature 	mozilla::safebrowsing::SafebrowsingHash<int, mozilla::safebrowsing::CompletionComparator>::FromPlaintext(nsACString_internal const&, nsICryptoHash*) More Reports Search
UUID	6d590738-66ba-410c-b214-1af632121003
Date Processed	2012-10-03 08:55:18
Uptime	3
Last Crash	11 seconds before submission
Install Age	15.7 minutes since version was first installed.
Install Time	2012-10-03 08:39:27
Product	Firefox
Version	18.0a1
Build ID	20121002030526
Release Channel	nightly
OS	Windows NT
OS Version	5.1.2600 Service Pack 3
Build Architecture	x86
Build Architecture Info	GenuineIntel family 15 model 6 stepping 5
Crash Reason	EXCEPTION_ACCESS_VIOLATION_READ
Crash Address	0x0
App Notes 
AdapterVendorID: 0x8086, AdapterDeviceID: 0x2772, AdapterSubsysID: 26331019, AdapterDriverVersion: 6.14.10.4926
D3D10 Layers? D3D10 Layers- 
EMCheckCompatibility	True
Adapter Vendor ID	0x8086
Adapter Device ID	0x2772
Total Virtual Memory	2147352576
Available Virtual Memory	1965834240
System Memory Use Percentage	56
Available Page File	893042688
Available Physical Memory	230576128

Frame 	Module 	Signature 	Source
0 	xul.dll 	mozilla::safebrowsing::SafebrowsingHash<32,mozilla::safebrowsing::CompletionComp 	toolkit/components/url-classifier/Entries.h:46
1 	xul.dll 	mozilla::safebrowsing::Classifier::Check 	toolkit/components/url-classifier/Classifier.cpp:304
2 	xul.dll 	nsUrlClassifierDBServiceWorker::DoLookup 	toolkit/components/url-classifier/nsUrlClassifierDBService.cpp:298
3 	xul.dll 	nsUrlClassifierDBServiceWorker::HandlePendingLookups 	toolkit/components/url-classifier/nsUrlClassifierDBService.cpp:346
4 	nspr4.dll 	_MD_CURRENT_THREAD 	nsprpub/pr/src/md/windows/w95thred.c:312
5 	xul.dll 	nsUrlClassifierDBServiceWorker::Lookup 	toolkit/components/url-classifier/nsUrlClassifierDBService.cpp:390
6 	xul.dll 	UrlClassifierDBServiceWorkerProxy::LookupRunnable::Run 	toolkit/components/url-classifier/nsUrlClassifierProxies.cpp:33
7 	xul.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:612
8 	xul.dll 	nsThread::ThreadFunc 	xpcom/threads/nsThread.cpp:256
9 	nspr4.dll 	_PR_NativeRunThread 	nsprpub/pr/src/threads/combined/pruthr.c:395
10 	nspr4.dll 	pr_root 	nsprpub/pr/src/md/windows/w95thred.c:90
11 	msvcr100.dll 	_callthreadstartex 	f:\dd\vctools\crt_bld\self_x86\crt\src\threadex.c:314
12 	msvcr100.dll 	_threadstartex 	f:\dd\vctools\crt_bld\self_x86\crt\src\threadex.c:292
13 	kernel32.dll 	BaseThreadStart 	

More reports at:
https://crash-stats.mozilla.com/report/list?signature=mozilla%3A%3Asafebrowsing%3A%3ASafebrowsingHash%3Cint%2C+mozilla%3A%3Asafebrowsing%3A%3ACompletionComparator%3E%3A%3AFromPlaintext%28nsACString_internal+const%26%2C+nsICryptoHash*%29

Comment 1

[Marcia Knous [:marcia]]

Adding [@ mozilla::safebrowsing::Classifier::Check ], which is the Mac and Linux specific signature.

Comment 2

[Scoobidiver (away)]

Firefox for Android is also affected.

More reports at:
https://crash-stats.mozilla.com/report/list?signature=mozilla%3A%3Asafebrowsing%3A%3AClassifier%3A%3ACheck
https://crash-stats.mozilla.com/report/list?signature=mozilla%3A%3Asafebrowsing%3A%3AClassifier%3A%3ACheck%28nsACString_internal+const%26%2C+nsTArray%3Cmozilla%3A%3Asafebrowsing%3A%3ALookupResult%2C+nsTArrayDefaultAllocator%3E%26%29

Comment 3

[Gian-Carlo Pascutto [:gcp]]

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/url-classifier/nsUrlClassifierDBService.cpp#780

If opening the Classifier fails, we give a warning and continue. 

http://mxr.mozilla.org/mozilla-central/source/toolkit/components/url-classifier/Classifier.cpp#198

This could cause us to bail out of Open without it being initialized.  Not being able to initialize the Classifier should be a catastrophic failure. The question is why it's happening, we'll likely need additional info for that but this crash might be fixable.

Comment 4

[Scoobidiver (away)]

It's #16 top browser crasher in 17.0b1 and #24 in 18.0a2.

Correlations are similar to the ones in bug 798778.

Comment 5

[Loic]

Some user here (http://forums.mozillazine.org/viewtopic.php?f=23&t=2574971) got these crashes too. And he's able to repro the issue.

Comment 6

[Gian-Carlo Pascutto [:gcp]]

>Some user here (http://forums.mozillazine.org/viewtopic.php?f=23&t=2574971) got 
>these crashes too. And he's able to repro the issue.

I posted in that thread but it said it needs moderator approval.

Anyway: having urlclassifier3.sqlite, urlclassifierkey3.txt, urlclassifier.pset and the safebrowsing subdir from the users (Local) profile might help in reproducing this. It would help if the user can post in this bug. Do not reset the profile without backing up first - having a "corrupted" profile that reproducibly creates this bug would be very useful for us to investigate this issue.

Comment 7

[Gian-Carlo Pascutto [:gcp]]

Attachment: Patch 1. Make sure we initialize completely or fail completely

https://tbpl.mozilla.org/?tree=Try&rev=b2fb707e9146

Comment 8

[Jim]

Per this thread in Forums http://forums.mozillazine.org/viewtopic.php?f=23&t=2574971&p=12385001#p12385001 I had this problem. I had a series of websites in two sets of Tab Groups. I bookmarked these two sets of sites in 2 sets of folders and deleted / closed the 2 Tab Groups leaving the main group. Once that was done the problem with Aurora crashing on each start up did not occur. the following are the two set of sites that were in these 2 Tab Groups:

Group1
about:blank
http://store.apple.com/us
http://www.griffintechnology.com/business/multidock
http://teachwithyouripad.wikispaces.com/Sync-Charge+Cart+and+Station+Comparison

Group 2
http://www.memoryexpress.com/Products/MX30682
http://forums.pcper.com/showthread.php?479478-Mid-Range-Build-Gigabyte-s-GA-Z68AP-D3-doesn-t-work-w-Ivy-Bridge
http://forums.anandtech.com/showthread.php?t=2274106
http://www.elotouch.com/Solutions/Retail/pos.asp
https://www.google.ca/search?q=%22retail+star%22+%22touch+screen%22&hl=en&client=firefox-beta&rls=org.mozilla:en-GB:official&channel=fflb&prmd=imvns&ei=Yp1sUJKPEYikyQHo74HIBw&start=20&sa=N&biw=1680&bih=912&bav=on.2,or.r_gc.r_pw.r_cp.r_qf.&cad=b&sei=QJtsUNeYIKqVyAG42YCoCw&ech=1&psi=QJtsUNeYIKqVyAG42YCoCw.1349359472493.3&emsg=NCSR&noj=1&ei=QJtsUNeYIKqVyAG42YCoCw

Comment 9

[Gian-Carlo Pascutto [:gcp]]

Jim, I tried to reproduce this with your instructions but failed. 

This bug would happen if there is something wrong with the profile that causes some Firefox subsystems to fail to initialize, which is somewhat consistent with your description of your profile behaving strange when moving between Firefox versions.

I might be able to infer what's wrong from the files described in comment 6, from a profile that, at the moment the files are achieved, is actively crashing.

Comment 10

[Gian-Carlo Pascutto [:gcp]]

Attachment: Patch 1. v2 Make sure we initialize completely or fail completely

Some additions:
1) Make sure we display a warning and halt debug builds when this happens.
2) The OpenDb code checks mClassifier !nullness and skips if true. So that should be the last thing the function does, else we risk a similar bug (partial init) popping up there too.

Comment 11

[Jim]

I do have a follow up question regarding the problem that I was seeing. The problem only occurred if I tried to use Aurora 18. Going between 16.0.1 and Beta17 I had no problem and removing the two Tab groups was all that I did behind the scenes. I could restore these tab groups but if Firefox starts crashing again I would need to switch to a different browser to post an upgate as well as to preserve the files.

As mentioned in my initial forum post over on the Mozillazine I have these three versions of Firefox all installed on the same machine, using the same profile directory. Is this frowned upon and should one solely rely on Firefox Sync to keep the current tabs opened between sessions, browser history, bookmarks etc on the same computer? Although in this case would Sync even be suitable to share information about multiple version installs of Firefox?

Regarding Sync, I always have difficulty in configuring what order and on which computers the initial setup steps have to be performed and frequently see sync complainining about unable to sync.

Comment 12

[Gian-Carlo Pascutto [:gcp]]

>I could restore these tab groups but if Firefox starts crashing again I would need 
>to switch to a different browser to post an upgate as well as to preserve the 
>files.

You can archive the files (or maybe the entire profile dir) in Windows Explorer by copying and zipping them to somewhere else, and then fix your problem. No need to keep the browser in the broken state. (I suspect there's a good possibility that you might not be able to reproduce the crashes, to be honest)

>Is this frowned upon and should one solely rely on Firefox Sync to keep the 
>current tabs opened between sessions, browser history, bookmarks etc on the same 
>computer? Although in this case would Sync even be suitable to share information 
>about multiple version installs of Firefox?

Sync supports multiple Firefox versions and even totally different platforms. i.e. You can sync between a Windows desktop on 18 to an Android tablet on 17 and a Mac machine on 16.

The problem with using Sync in a setup like yours is that you would need to use seperate profiles for each browser, i.e. select the right profile on each startup. That's probably not going to be very convenient.

In my experience Firefox generally works fine when upgrading/downgrading on the same profile (I do it all the time during development/testing), though I'm not sure how much this is officially supported, particularly in the downgrading direction. I'm also not sure if anyone who could give you a definite answer on that is necessarily reading this bug.

I would generally advise you that if you have a setup that works well for you, to keep it. If you run into a problem that makes Firefox crash, as you did here, you're welcome to file bugs in Bugzilla as we're interested in fixing any "crasher" problems.

Comment 13

[Gian-Carlo Pascutto [:gcp]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/ecc0cdaf3580

Comment 14

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 673192 [details] [diff] [review]
Patch 1. v2 Make sure we initialize completely or fail completely

[Approval Request Comment]
Bug caused by (feature/regressing bug #): Bug 673470
User impact if declined: Startup crash if the profile is corrupted or unwritable. (Likely also fixes bug 798778)
Testing completed (on m-c, etc.): Only just landed.
Risk to taking this patch (and alternatives if risky): Reasonably low as it mostly affects error handling.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/ecc0cdaf3580

Comment 17

[Alex Keybl [:akeybl]]

Comment on attachment 673192 [details] [diff] [review]
Patch 1. v2 Make sure we initialize completely or fail completely

Approving for Aurora 18, but let's make sure this has lowers the top crash volume before uplifting to Beta.

Comment 18

[Gian-Carlo Pascutto [:gcp]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/8e856706c705

Comment 19

[Gian-Carlo Pascutto [:gcp]]

nsUrlClassifierDBServiceWorker::FinishUpdate() and mozilla::safebrowsing::SafebrowsingHash crash signatures dropped 12 places in the topcrasher list.

Comment 20

[Robert Kaiser]

(In reply to Gian-Carlo Pascutto (:gcp) from comment #19)
> nsUrlClassifierDBServiceWorker::FinishUpdate() and
> mozilla::safebrowsing::SafebrowsingHash crash signatures dropped 12 places
> in the topcrasher list.

Of which one? I.e. the one of which version of Firefox? One where the patch landed? If so, that's good as the bug 798778 signatures is rising on 17 so if the patch helps, it would be good to get it in there.

Comment 21

[Gian-Carlo Pascutto [:gcp]]

I was looking at the overall Aurora 18 statistics, where this patch landed a few days ago. (I was replying to comment 17)

Comment 22

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 673192 [details] [diff] [review]
Patch 1. v2 Make sure we initialize completely or fail completely

Looks good - let's get this in on Beta branch today so we've got this in tomorrow morning's Beta builds and we can collect the data on Beta channel over the coming week to confirm decrease in crashes there.

Comment 23

[Gian-Carlo Pascutto [:gcp]]

https://hg.mozilla.org/releases/mozilla-beta/rev/0677438ae4ce