Last Comment Bug 797302 - crash in mozilla::safebrowsing::Classifier::Check
: crash in mozilla::safebrowsing::Classifier::Check
Status: RESOLVED FIXED
[native-crash][startupcrash]
: crash, regression, topcrash
Product: Toolkit
Classification: Components
Component: Safe Browsing (show other bugs)
: 17 Branch
: All All
: -- critical (vote)
: Firefox 19
Assigned To: Gian-Carlo Pascutto [:gcp]
:
Mentors:
: 614582 798778 (view as bug list)
Depends on:
Blocks: 673470
  Show dependency treegraph
 
Reported: 2012-10-03 02:51 PDT by Scoobidiver (away)
Modified: 2014-05-27 12:25 PDT (History)
9 users (show)
ryanvm: in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
fixed
+
fixed


Attachments
Patch 1. Make sure we initialize completely or fail completely (1.50 KB, patch)
2012-10-17 08:27 PDT, Gian-Carlo Pascutto [:gcp]
no flags Details | Diff | Splinter Review
Patch 1. v2 Make sure we initialize completely or fail completely (3.54 KB, patch)
2012-10-19 05:07 PDT, Gian-Carlo Pascutto [:gcp]
dcamp: review+
akeybl: approval‑mozilla‑aurora+
lukasblakk+bugs: approval‑mozilla‑beta+
Details | Diff | Splinter Review

Description Scoobidiver (away) 2012-10-03 02:51:50 PDT
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 - use ni] 2012-10-16 07:09:17 PDT
Adding [@ mozilla::safebrowsing::Classifier::Check ], which is the Mac and Linux specific signature.
Comment 3 Gian-Carlo Pascutto [:gcp] 2012-10-16 07:27:04 PDT
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) 2012-10-16 07:34:39 PDT
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 2012-10-17 05:32:49 PDT
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] 2012-10-17 08:06:26 PDT
>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] 2012-10-17 08:27:41 PDT
Created attachment 672317 [details] [diff] [review]
Patch 1. Make sure we initialize completely or fail completely

https://tbpl.mozilla.org/?tree=Try&rev=b2fb707e9146
Comment 9 Gian-Carlo Pascutto [:gcp] 2012-10-19 01:59:30 PDT
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] 2012-10-19 05:07:57 PDT
Created attachment 673192 [details] [diff] [review]
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 2012-10-19 07:44:26 PDT
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] 2012-10-22 00:02:47 PDT
>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] 2012-10-23 00:58:12 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/ecc0cdaf3580
Comment 14 Gian-Carlo Pascutto [:gcp] 2012-10-23 01:01:33 PDT
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 15 Gian-Carlo Pascutto [:gcp] 2012-10-23 01:10:23 PDT
*** Bug 798778 has been marked as a duplicate of this bug. ***
Comment 16 Ryan VanderMeulen [:RyanVM] 2012-10-23 20:00:54 PDT
https://hg.mozilla.org/mozilla-central/rev/ecc0cdaf3580
Comment 17 Alex Keybl [:akeybl] 2012-10-24 15:36:30 PDT
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] 2012-10-25 02:23:15 PDT
https://hg.mozilla.org/releases/mozilla-aurora/rev/8e856706c705
Comment 19 Gian-Carlo Pascutto [:gcp] 2012-10-29 07:51:00 PDT
nsUrlClassifierDBServiceWorker::FinishUpdate() and mozilla::safebrowsing::SafebrowsingHash crash signatures dropped 12 places in the topcrasher list.
Comment 20 Robert Kaiser 2012-10-29 09:23:38 PDT
(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] 2012-10-29 09:36:26 PDT
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 2012-10-29 10:20:37 PDT
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] 2012-10-29 11:08:59 PDT
https://hg.mozilla.org/releases/mozilla-beta/rev/0677438ae4ce
Comment 24 Gian-Carlo Pascutto [:gcp] 2013-02-28 00:09:42 PST
*** Bug 614582 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.