731187 - OOM Crash [@ gfxTextRun::GetAdvanceWidth] due to unhandled alloc failure of gfxTextRun::AllocateStorageForTextRun()

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect)

Description

[Christian Holler (:decoder)]

Tested on m-c revision 66e4d53697c2: The method gfxTextRun::AllocateStorageForTextRun() may fail to allocate and returns nsnull. The return value is stored in AutoTextRun and probably not checked later as it causes the following crash:

WARNING: failed to allocate storage for text run!: file /srv/repos/browser/mozilla-central/gfx/thebes/gfxFont.cpp, line 4059

Program received signal SIGSEGV, Segmentation fault.
gfxTextRun::GetAdvanceWidth (this=0x0, aStart=0, aLength=4, aProvider=0x7fffffff9870) at /srv/repos/browser/mozilla-central/gfx/thebes/gfxFont.cpp:4795
4795        NS_ASSERTION(aStart + aLength <= mCharacterCount, "Substring out of range");
#0  gfxTextRun::GetAdvanceWidth (this=0x0, aStart=0, aLength=4, aProvider=0x7fffffff9870) at /srv/repos/browser/mozilla-central/gfx/thebes/gfxFont.cpp:4795
#1  0x00002aaaac640948 in nsFontMetrics::GetWidth (this=<optimized out>, aString=<optimized out>, aLength=4, aContext=<optimized out>) at /srv/repos/browser/mozilla-central/gfx/src/nsFontMetrics.cpp:318
#2  0x00002aaaac63fd46 in nsRenderingContext::GetWidth (this=0x46f9c70, aString=<optimized out>, aLength=<optimized out>) at /srv/repos/browser/mozilla-central/gfx/src/nsRenderingContext.cpp:507
#3  0x00002aaaac694b0e in nsLayoutUtils::GetStringWidth (aFrame=0x4cab530, aContext=0x46f9c70, aString=0x2ea85e8, aLength=4) at /srv/repos/browser/mozilla-central/layout/base/nsLayoutUtils.cpp:3105
#4  0x00002aaaac8220bc in nsTextBoxFrame::GetTextSize (this=0x4cab530, aPresContext=<optimized out>, aRenderingContext=..., aString=..., aSize=..., aAscent=<optimized out>) at /srv/repos/browser/mozilla-central/layout/xul/base/src/nsTextBoxFrame.cpp:978
#5  0x00002aaaac822132 in CalcTextSize (aBoxLayoutState=<optimized out>, this=0x4cab530) at /srv/repos/browser/mozilla-central/layout/xul/base/src/nsTextBoxFrame.cpp:992
#6  nsTextBoxFrame::CalcTextSize (this=0x4cab530, aBoxLayoutState=<optimized out>) at /srv/repos/browser/mozilla-central/layout/xul/base/src/nsTextBoxFrame.cpp:983
#7  0x00002aaaac822237 in nsTextBoxFrame::GetPrefSize (this=0x4cab530, aBoxLayoutState=<optimized out>) at /srv/repos/browser/mozilla-central/layout/xul/base/src/nsTextBoxFrame.cpp:1042


The backtrace of the failing allocation is as follows:

#0 /srv/repos/browser/mozilla-central/objdir-ff-gcc64dbg/dist/bin/libmozalloc.so(moz_malloc+0x5f) [0x2aaaaab2415c] (aab2415c)
#1 gfxTextRun::AllocateStorageForTextRun(unsigned long, unsigned int) at gfx/thebes/gfxFont.cpp:4058
#2 gfxTextRun::Create(gfxTextRunFactory::Parameters const*, void const*, unsigned int, gfxFontGroup*, unsigned int) at gfx/thebes/gfxFont.cpp:4075
#3 gfxFontGroup::MakeTextRun(unsigned short const*, unsigned int, gfxTextRunFactory::Parameters const*, unsigned int) at gfx/thebes/gfxFont.cpp:3082
#4 AutoTextRun at gfx/src/nsFontMetrics.cpp:66
#5 nsFontMetrics::GetWidth(unsigned short const*, unsigned int, nsRenderingContext*) at gfx/src/nsFontMetrics.cpp:318
#6 nsRenderingContext::GetWidth(unsigned short const*, unsigned int) at gfx/src/nsRenderingContext.cpp:507
#7 nsLayoutUtils::GetStringWidth(nsIFrame const*, nsRenderingContext*, unsigned short const*, int) at layout/base/nsLayoutUtils.cpp:3106
#8 nsTextBoxFrame::GetTextSize(nsPresContext*, nsRenderingContext&, nsString const&, nsSize&, int&) at layout/xul/base/src/nsTextBoxFrame.cpp:979
#9 nsTextBoxFrame::CalcTextSize(nsBoxLayoutState&) at layout/xul/base/src/nsTextBoxFrame.cpp:993
#10 nsTextBoxFrame::GetPrefSize(nsBoxLayoutState&) at layout/xul/base/src/nsTextBoxFrame.cpp:1044
#11 nsSprocketLayout::GetPrefSize(nsIFrame*, nsBoxLayoutState&) at layout/xul/base/src/nsSprocketLayout.cpp:1366
#12 nsBoxFrame::GetPrefSize(nsBoxLayoutState&) at layout/xul/base/src/nsBoxFrame.cpp:791
#13 nsMenuFrame::GetPrefSize(nsBoxLayoutState&) at layout/xul/base/src/nsMenuFrame.cpp:1364
#14 nsSprocketLayout::GetPrefSize(nsIFrame*, nsBoxLayoutState&) at layout/xul/base/src/nsSprocketLayout.cpp:1366
#15 nsBoxFrame::GetPrefSize(nsBoxLayoutState&) at layout/xul/base/src/nsBoxFrame.cpp:791

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: patch, make AutoTextRun check for failure to create the run

Yes, it looks like AutoTextRun doesn't handle failure to create the run. I guess the various nsFontMetrics methods should just bail and return 0 or do nothing in this case.

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: patch v2, make AutoTextRun check for failure to create the run

Oops, first version didn't actually build. This should be better.

Comment 3

[Christian Holler (:decoder)]

Comment on attachment 601283 [details] [diff] [review]
patch v2, make AutoTextRun check for failure to create the run

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

Verified that the crash on OOM here is gone. The browser looks pretty ugly with all ::Create calls failing due to OOM, but even when browsing, it remained stable and didn't crash ^_^.

Just fyi, not sure if my review here is worth anything as I'm not a peer for any module in Firefox :) But I can confirm it fixes the issue.

Comment 4

[Jonathan Kew [:jfkthame]]

Comment on attachment 601283 [details] [diff] [review]
patch v2, make AutoTextRun check for failure to create the run

OK, flagging roc for additional review. Thanks for testing! I'm sure it'd look pretty awful with lots of text missing, but if we can't get enough memory to create the textruns, I think that's a reasonable outcome. Shouldn't happen in normal use, of course - I assume you're using some kind of extreme stress test to hit this.

Comment 5

[Jonathan Kew [:jfkthame]]

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

Comment 6

[Matt Brubeck (:mbrubeck)]

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