1435859 - Crash in OOM | large | NS_ABORT_OOM | mozilla::safebrowsing::Classifier::DeleteTables

Status: RESOLVED FIXED

(Toolkit :: Safe Browsing, defect, P1)

Description

[[:philipp]]

This bug was filed from the Socorro interface and is
report bp-0086eab2-59ed-42a3-bf7a-8cfd60180205.
=============================================================

Top 10 frames of crashing thread:

0 xul.dll NS_ABORT_OOM xpcom/base/nsDebugImpl.cpp:614
1 xul.dll mozilla::safebrowsing::Classifier::DeleteTables toolkit/components/url-classifier/Classifier.cpp:362
2 xul.dll mozilla::safebrowsing::Classifier::ResetTables toolkit/components/url-classifier/Classifier.cpp:326
3 xul.dll mozilla::safebrowsing::Classifier::ApplyUpdatesForeground toolkit/components/url-classifier/Classifier.cpp:872
4 xul.dll <lambda_506e6a83a22d85e62e5b05da709b58c7>::operator toolkit/components/url-classifier/Classifier.cpp:755
5 xul.dll mozilla::detail::RunnableFunction<<lambda_506e6a83a22d85e62e5b05da709b58c7> >::Run xpcom/threads/nsThreadUtils.h:529
6 xul.dll nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:1040
7 xul.dll NS_ProcessNextEvent xpcom/threads/nsThreadUtils.cpp:517
8 xul.dll mozilla::ipc::MessagePumpForNonMainThreads::Run ipc/glue/MessagePump.cpp:364
9 xul.dll MessageLoop::RunHandler ipc/chromium/src/base/message_loop.cc:319

=============================================================

these oom crash reports on windows are regressing in firefox 58 & happen both on 32bit and win64 builds. the recorded memory allocation size always seems to be exactly 4 .GiB

Comment 1

[François Marier [:francois]]

Attachment: Bug 1435859 - Fix OOM crash on filenames without extensions.

Passing a value of -1 to nsCString::Truncate() converts that
value to a large integer and leads to an unnecessary 4GB
memory allocation.

Review commit: https://reviewboard.mozilla.org/r/220694/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/220694/

Comment 2

[Gian-Carlo Pascutto [:gcp]]

Comment on attachment 8951412 [details]
Bug 1435859 - Fix OOM crash on filenames without extensions.

https://reviewboard.mozilla.org/r/220694/#review227860

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d418ce8a0564
Fix OOM crash on filenames without extensions. r=gcp

Comment 4

[Dorel Luca [:dluca]]

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

Comment 5

[Ryan VanderMeulen [:RyanVM]]

Pretty low-volume, but the fix looks pretty simple and straightforward. What do you think about uplifting this to 59?

Comment 6

[François Marier [:francois]]

Comment on attachment 8951412 [details]
Bug 1435859 - Fix OOM crash on filenames without extensions.

Approval Request Comment
[Feature/Bug causing the regression]: Bug 1435859
[User impact if declined]: OOM crash under certain circumstances during Safe Browsing updates
[Is this code covered by automated tests?]: No
[Has the fix been verified in Nightly?]: No
[Needs manual test from QE? If yes, steps to reproduce]: Probably not necessary.
[List of other uplifts needed for the feature/fix]: None
[Is the change risky?]: No
[Why is the change risky/not risky?]: It's a very straightforward fix.
[String changes made/needed]: None

Comment 7

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8951412 [details]
Bug 1435859 - Fix OOM crash on filenames without extensions.

Simple fix for an OOM crash caused by our code doing bad things on filenames without extensions. Taking for 59b13.

Comment 8

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/17c5d6591dde