1345815 - Crash in OOM | large | mozalloc_abort | mozalloc_handle_oom | moz_xmalloc | mozilla::layers::FillRectWithMask

Status: RESOLVED FIXED

(Core :: Graphics: Layers, defect, P1)

Description

[[:philipp]]

This bug was filed from the Socorro interface and is 
report bp-2a9ef9a0-13c6-4b21-b266-a528b2170309.
=============================================================
Crashing Thread (0)
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 	mozilla::layers::FillRectWithMask(mozilla::gfx::DrawTarget*, mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float> const&, mozilla::gfx::SourceSurface*, mozilla::gfx::SamplingFilter, mozilla::gfx::DrawOptions const&, mozilla::gfx::ExtendMode, mozilla::gfx::SourceSurface*, mozilla::gfx::Matrix const*, mozilla::gfx::Matrix const*) 	gfx/layers/basic/BasicLayersImpl.cpp:169
4 	xul.dll 	mozilla::layers::FillRectWithMask(mozilla::gfx::DrawTarget*, mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits, float> const&, mozilla::gfx::RectTyped<mozilla::gfx::UnknownUnits, float> const&, mozilla::gfx::SourceSurface*, mozilla::gfx::SamplingFilter, mozilla::gfx::DrawOptions const&, mozilla::layers::Layer*) 	gfx/layers/basic/BasicLayersImpl.cpp:205
5 	xul.dll 	mozilla::layers::BasicCanvasLayer::Paint(mozilla::gfx::DrawTarget*, mozilla::gfx::PointTyped<mozilla::gfx::UnknownUnits, float> const&, mozilla::layers::Layer*) 	gfx/layers/basic/BasicCanvasLayer.cpp:117
6 	xul.dll 	mozilla::layers::BasicLayerManager::PaintSelfOrChildren(mozilla::layers::PaintLayerContext&, gfxContext*) 	gfx/layers/basic/BasicLayerManager.cpp:716
7 	xul.dll 	mozilla::layers::BasicLayerManager::PaintLayer(gfxContext*, mozilla::layers::Layer*, void (*)(mozilla::layers::PaintedLayer*, gfxContext*, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::layers::DrawRegionClip, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, void*), void*) 	gfx/layers/basic/BasicLayerManager.cpp:896
8 	xul.dll 	mozilla::layers::BasicLayerManager::PaintSelfOrChildren(mozilla::layers::PaintLayerContext&, gfxContext*) 	gfx/layers/basic/BasicLayerManager.cpp:736
9 	xul.dll 	mozilla::layers::BasicLayerManager::PaintLayer(gfxContext*, mozilla::layers::Layer*, void (*)(mozilla::layers::PaintedLayer*, gfxContext*, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::layers::DrawRegionClip, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, void*), void*) 	gfx/layers/basic/BasicLayerManager.cpp:896
10 	xul.dll 	mozilla::layers::BasicLayerManager::PaintSelfOrChildren(mozilla::layers::PaintLayerContext&, gfxContext*) 	gfx/layers/basic/BasicLayerManager.cpp:736
11 	xul.dll 	mozilla::layers::BasicLayerManager::PaintLayer(gfxContext*, mozilla::layers::Layer*, void (*)(mozilla::layers::PaintedLayer*, gfxContext*, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::layers::DrawRegionClip, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, void*), void*) 	gfx/layers/basic/BasicLayerManager.cpp:896
12 	xul.dll 	mozilla::layers::BasicLayerManager::EndTransactionInternal(void (*)(mozilla::layers::PaintedLayer*, gfxContext*, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, mozilla::layers::DrawRegionClip, mozilla::gfx::IntRegionTyped<mozilla::gfx::UnknownUnits> const&, void*), void*, mozilla::layers::LayerManager::EndTransactionFlags) 	gfx/layers/basic/BasicLayerManager.cpp:622
13 	xul.dll 	nsDisplayList::PaintRoot(nsDisplayListBuilder*, nsRenderingContext*, unsigned int) 	layout/painting/nsDisplayList.cpp:2055
14 	xul.dll 	nsLayoutUtils::PaintFrame(nsRenderingContext*, nsIFrame*, nsRegion const&, unsigned int, nsDisplayListBuilderMode, nsLayoutUtils::PaintFrameFlags) 	layout/base/nsLayoutUtils.cpp:3675
15 	xul.dll 	nsSimplePageSequenceFrame::PrintNextPage() 	layout/generic/nsSimplePageSequenceFrame.cpp:772
16 	xul.dll 	nsPrintEngine::PrintPage(nsPrintObject*, bool&) 	layout/printing/nsPrintEngine.cpp:2745
17 	xul.dll 	nsPagePrintTimer::Run() 	layout/printing/nsPagePrintTimer.cpp:89
18 	xul.dll 	nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp:1240
19 	xul.dll 	mozilla::ipc::MessagePump::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp:96
20 	xul.dll 	mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp:301
21 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc:231
22 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc:211
23 	xul.dll 	nsBaseAppShell::Run() 	widget/nsBaseAppShell.cpp:156
24 	xul.dll 	nsAppShell::Run() 	widget/windows/nsAppShell.cpp:262
25 	xul.dll 	XRE_RunAppShell() 	toolkit/xre/nsEmbedFunctions.cpp:924
26 	xul.dll 	mozilla::ipc::MessagePumpForChildProcess::Run(base::MessagePump::Delegate*) 	ipc/glue/MessagePump.cpp:269
27 	xul.dll 	MessageLoop::RunHandler() 	ipc/chromium/src/base/message_loop.cc:231
28 	xul.dll 	MessageLoop::Run() 	ipc/chromium/src/base/message_loop.cc:211
29 	xul.dll 	XRE_InitChildProcess(int, char** const, XREChildData const*) 	toolkit/xre/nsEmbedFunctions.cpp:756
30 	xul.dll 	mozilla::BootstrapImpl::XRE_InitChildProcess(int, char** const, XREChildData const*) 	toolkit/xre/Bootstrap.cpp:65
31 	firefox.exe 	content_process_main(mozilla::Bootstrap*, int, char** const) 	ipc/contentproc/plugin-container.cpp:115
32 	firefox.exe 	wmain 	toolkit/xre/nsWindowsWMain.cpp:115
33 	firefox.exe 	__scrt_common_main_seh 	f:/dd/vctools/crt/vcstartup/src/startup/exe_common.inl:253
34 	kernel32.dll 	BaseThreadInitThunk 	
35 	ntdll.dll 	__RtlUserThreadStart 	
36 	ntdll.dll 	_RtlUserThreadStart

these out-of-memory signatures on windows seem to be regressing since 53 and later (related to bug 1311512?).
so far in very early crash data from 53.0b1 they account for 1.6% of all crashes there

Comment 1

[Lee Salzman [:lsalzman]]

Tobias, it looks like you added an allocation there for recording playback, as part of bug 1311512, that should be made fallible so as to not crash like this in the future?

Also, shortly after the new you copy the contents into a std::string, which rather than merely reference the buffer, does an allocation and a copy, introducing a fairly exorbitant cost for large buffers and introducing another potential source of OOMs, since that would also need to be made fallible?

Comment 2

[Lee Salzman [:lsalzman]]

Attachment: fix BasicLayers OOM with recording playback by avoiding copies

Since the underlying stream here is an iostream, it seems like the best course of action here is to not actually copy it at all and just use it directly as an istream. That way, there is no chance of OOM or even failure here.

Comment 3

[Tobias Schneider [:tobytailor]]

Comment on attachment 8845535 [details] [diff] [review]
fix BasicLayers OOM with recording playback by avoiding copies

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

This is looking good to me. Thanks for the patch!

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/664bbd7ad4d5
fix BasicLayers OOM with recording playback by avoiding copies. r=tobytailor

Comment 5

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/664bbd7ad4d5

Comment 6

[Peter Chang[:pchang]]

Lee, could you request uplift as well?

Comment 7

[Lee Salzman [:lsalzman]]

Comment on attachment 8845535 [details] [diff] [review]
fix BasicLayers OOM with recording playback by avoiding copies

Approval Request Comment
[Feature/Bug causing the regression]: bug 1311512
[User impact if declined]: OOMs during printing.
[Is this code covered by automated tests?]: no
[Has the fix been verified in Nightly?]: yes
[Needs manual test from QE? If yes, steps to reproduce]: no 
[List of other uplifts needed for the feature/fix]: 53, 54
[Is the change risky?]: no
[Why is the change risky/not risky?]: Avoids doing an allocation where none was necessary during printing.
[String changes made/needed]: None

Comment 8

[Julien Cristau [:jcristau]]

Comment on attachment 8845535 [details] [diff] [review]
fix BasicLayers OOM with recording playback by avoiding copies

fix oom regression in aurora54 and beta53

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/0b07fe8eb11d

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/0aa0fdf91411

Comment 11

[Andrei Vaida [:avaida]]

Setting qe-verify- based on Lee's assessment on manual testing needs (see Comment 7).