Closed Bug 1276724 Opened 3 years ago Closed 3 years ago

Crash in OOM | large | mozalloc_abort | mozalloc_handle_oom | moz_xmalloc | nsTArray_base<T>::EnsureCapacity<T> | mozilla::safebrowsing::HashStore::WriteAddPrefixes and elsewhere in url-classifier

Categories

(Toolkit :: Safe Browsing, defect, P1, critical)

defect

Tracking

()

RESOLVED FIXED
mozilla50
Tracking Status
firefox46 --- wontfix
firefox47 --- wontfix
firefox48 --- wontfix
firefox49 --- fixed
firefox-esr45 - wontfix
firefox50 --- fixed
firefox53 --- fixed
firefox54 --- fixed
firefox55 --- fixed

People

(Reporter: philipp, Assigned: njn)

References

Details

(Keywords: crash)

Crash Data

Attachments

(1 file)

This bug was filed from the Socorro interface and is 
report bp-9237bb75-6fef-48e3-a659-42cc82160523.
=============================================================
Crashing Thread (48)
Frame 	Module 	Signature 	Source
0 	mozglue.dll 	mozalloc_abort(char const* const) 	memory/mozalloc/mozalloc_abort.cpp:33
1 	mozglue.dll 	mozalloc_handle_oom(unsigned int) 	memory/mozalloc/mozalloc_oom.cpp:46
2 	mozglue.dll 	moz_xmalloc 	memory/mozalloc/mozalloc.cpp:85
3 	xul.dll 	nsTArray_base<nsTArrayInfallibleAllocator, nsTArray_CopyWithMemutils>::EnsureCapacity<nsTArrayInfallibleAllocator>(unsigned int, unsigned int) 	xpcom/glue/nsTArray-inl.h:136
4 	xul.dll 	mozilla::safebrowsing::HashStore::WriteAddPrefixes(nsIOutputStream*) 	toolkit/components/url-classifier/HashStore.cpp:777
5 	xul.dll 	mozilla::safebrowsing::Classifier::ApplyTableUpdates(nsTArray<mozilla::safebrowsing::TableUpdate*>*, nsACString_internal const&) 	toolkit/components/url-classifier/Classifier.cpp:663
6 	xul.dll 	nsUrlClassifierDBServiceWorker::CacheCompletions(nsTArray<mozilla::safebrowsing::CacheResult>*) 	toolkit/components/url-classifier/nsUrlClassifierDBService.cpp:700
7 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp:995

this oom crash seems to occur on various windows systems and on android as well. on 47.0b9 this signature is currently around #40 by volume.
Priority: -- → P1
Assignee: nobody → francois
Status: NEW → ASSIGNED
Crash volume for signature 'OOM | large | mozalloc_abort | mozalloc_handle_oom | moz_xmalloc | nsTArray_base<T>::EnsureCapacity<T> | mozilla::safebrowsing::HashStore::WriteAddPrefixes':
  - esr (45): 678

Affected platform: Windows
These crashes have occurred about 2300 times in the past 7 days across all versions of Firefox. Fixing them is easy.
Crash Signature: [@ OOM | large | mozalloc_abort | mozalloc_handle_oom | moz_xmalloc | nsTArray_base<T>::EnsureCapacity<T> | mozilla::safebrowsing::HashStore::WriteAddPrefixes] → [@ OOM | large | mozalloc_abort | mozalloc_handle_oom | moz_xmalloc | nsTArray_base<T>::EnsureCapacity<T> | mozilla::safebrowsing::HashStore::WriteAddPrefixes] [@ OOM | large | mozalloc_abort | mozalloc_handle_oom | moz_xmalloc | nsTArray_base<T>::Ensure…
Summary: Crash in OOM | large | mozalloc_abort | mozalloc_handle_oom | moz_xmalloc | nsTArray_base<T>::EnsureCapacity<T> | mozilla::safebrowsing::HashStore::WriteAddPrefixes → Crash in OOM | large | mozalloc_abort | mozalloc_handle_oom | moz_xmalloc | nsTArray_base<T>::EnsureCapacity<T> | mozilla::safebrowsing::HashStore::WriteAddPrefixes and elsewhere in url-classifier
This addresses two crashes in the top #75 on 47.0.1.
Attachment #8775848 - Flags: review?(gpascutto)
Assignee: francois → n.nethercote
Comment on attachment 8775848 [details] [diff] [review]
Make two url-classifier allocations fallible

Review of attachment 8775848 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM. The first failure is correctly detected upwards as OOM and won't result in a database reset, and the second one is before the old PrefixSet is cleared, so it won't silently disable SafeBrowsing.
Attachment #8775848 - Flags: review?(gpascutto) → review+
https://hg.mozilla.org/mozilla-central/rev/19c9fa346278
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
the signature is currently causing 0.25% of browser crashes on 49.0b.
could we uplift the patch to 49 in terms of the scope/risk?
Flags: needinfo?(n.nethercote)
Comment on attachment 8775848 [details] [diff] [review]
Make two url-classifier allocations fallible

Approval Request Comment

[Feature/regressing bug #]: safebrowsing

[User impact if declined]: OOM crashes, currently 0.25% of crashes on beta.

[Describe test coverage new/current, TreeHerder]: landed on m-c 13 days ago. No specific tests.

[Risks and why]: Very low. Patch is trivial, it just makes two allocations fallible. All callers to the two functions containing those allocations appropriately check for failure, so any OOM should be handled well. 

[String/UUID change made/needed]: none.
Flags: needinfo?(n.nethercote)
Attachment #8775848 - Flags: approval-mozilla-beta?
Comment on attachment 8775848 [details] [diff] [review]
Make two url-classifier allocations fallible

Crash fix, let's give it a try on beta.
Attachment #8775848 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Duplicate of this bug: 1188324
[Tracking Requested - why for this release]:

Signature report for OOM | large | mozalloc_abort | mozalloc_handle_oom | moz_xmalloc | je_free | mozilla::safebrowsing::LookupCache::ConstructPrefixSet

Showing results from 6 months ago

Operating System
Windows 7 	778 	81.9%
Windows 8.1 	67 	7.1%
Windows 10 	62 	6.5%
Windows Vista 	19 	2.0%
Windows XP 	17 	1.8%
Windows 8 	6 	0.6%
WindowsServer03	1 	0.1%

Product
Firefox	45.9.0esr 	20 	57.1% 	28
Firefox	45.3.0esr 	2 	5.7% 	2
Firefox	45.4.0esr 	2 	5.7% 	2
Firefox	45.5.1esr 	2 	5.7% 	2
Firefox	45.8.0esr 	2 	5.7% 	2
Firefox 	46.0.1 	2 	5.7% 	2
Firefox 	43.0.1 	1 	2.9% 	1
Firefox	45.6.0esr 	1 	2.9% 	1
Firefox 	47.0.1 	1 	2.9% 	1
Firefox 	47.0.2 	1 	2.9% 	1
Firefox	48.0b99 	1 	2.9% 	1

Architecture
x86 	950 	100.0%
You need to log in before you can comment on or make changes to this bug.