1070983 - crash mozalloc_abort in gfxFontGroup::BuildFontList() under OSX

Status: RESOLVED FIXED

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

Description

[John Daggett (:jtd)]

Example crash report:

mozalloc_abort(char const*) | Abort | NS_DebugBreak | gfxFontGroup::BuildFontList() More Reports Search

https://crash-stats.mozilla.com/report/index/8ddbe24c-a603-431b-924d-3fe012140915

This is basically "we can't find any fonts, even default ones, bailing...". So something is causing simple platform font lookups to fail.

I reworked some of this code for bug 280443 but this doesn't appear to be the cause as it was happening relatively frequently as far back as Firefox 30.

Firefox 30: 4 weeks < 22 July 2014  ==  5381 crashes
Firefox 31: 4 weeks < 22 Aug 2014   ==  4321 crashes
Firefox 32: 4 weeks < 22 Sept 2014  ==  2430 crashes

Comment 1

[John Daggett (:jtd)]

Doesn't seem OSX version related (using data from the crash report sets in description):

Firefox 30
Operating System: Provides a breakdown of the crashes by OS for version(s) and product(s). Operating System 	Percentage 	Number Of Crashes
OS X 10.9 	42.51 %	2274
OS X 10.6 	34.72 %	1857
OS X 10.7 	14.49 %	775
OS X 10.8 	8.21 %	439
OS X 10.10 	0.07 %

Firefox 31
Operating System 	Percentage 	Number Of Crashes
OS X 10.9 	42.84 %	1815
OS X 10.6 	34.55 %	1464
OS X 10.7 	13.97 %	592
OS X 10.8 	8.00 %	339
OS X 10.10 	0.64 %	27 

Firefox 32
Operating System 	Percentage 	Number Of Crashes
OS X 10.9 	50.48 %	1156
OS X 10.6 	27.73 %	635
OS X 10.7 	13.32 %	305
OS X 10.8 	7.82 %	179
OS X 10.10 	0.66 %	15

Comment 3

[John Daggett (:jtd)]

Clue as to one possible cause: in bug 1032072 the reporter ended up with a blank fontlist when installing a new font. 

I think we need to add some sort of annotation within the code of BuildFontList, so that if we do hit this we'll have some notion of what occurred within InitFontList just prior to that point.

I've tried fiddling with different things, but simply adding/deleting/enabling/disabling a font via FontBook does *not* trigger this.

Comment 4

[John Daggett (:jtd)]

The stacks for this under OSX indicate that the browser is getting notified about a font change. This results in the fontlist being initialized. However, as part of the process, gfxFontInfoLoader::CancelLoader() is called which shuts down the async font loader thread. This has the effect of spinning the event loop while all system fontlists have been cleared (!!!).

I think the solution is to farm the thread shutdown into a separate event so that the fontlists will always be in order when the event loop is spun. The only downside is that the existing fontloader thread might run a bit more before shutting down but that's certainly better than aborting!

https://crash-stats.mozilla.com/report/index/1bbdb75d-5113-4fea-ac42-7db262150729

Abort 	xpcom/base/nsDebugImpl.cpp
NS_DebugBreak 	xpcom/base/nsDebugImpl.cpp
gfxFontGroup::GetDefaultFont() 	gfx/thebes/gfxTextRun.cpp
void gfxFontGroup::InitScriptRun<unsigned char>(gfxContext*, gfxTextRun*, unsigned char const*, unsigned int, unsigned int, int, gfxMissingFontRecorder*) 	gfx/thebes/gfxTextRun.h
void gfxFontGroup::InitTextRun<unsigned char>(gfxContext*, gfxTextRun*, unsigned char const*, unsigned int, gfxMissingFontRecorder*) 	gfx/thebes/gfxTextRun.cpp
gfxFontGroup::MakeTextRun(unsigned char const*, unsigned int, gfxTextRunFactory::Parameters const*, unsigned int, gfxMissingFontRecorder*) 	gfx/thebes/gfxTextRun.cpp
gfxTextRun* MakeTextRun<unsigned char>(unsigned char const*, unsigned int, gfxFontGroup*, gfxTextRunFactory::Parameters const*, unsigned int, gfxMissingFontRecorder*) 	layout/generic/nsTextFrame.cpp
BuildTextRunsScanner::BuildTextRunForFrames(void*) 	layout/generic/nsTextFrame.cpp
BuildTextRunsScanner::FlushFrames(bool, bool) 	layout/generic/nsTextFrame.cpp
nsTextFrame::EnsureTextRun(nsTextFrame::TextRunType, gfxContext*, nsIFrame*, nsLineList_iterator const*, unsigned int*) 	layout/generic/nsTextFrame.cpp
nsTextFrame::AddInlineMinISizeForFlow(nsRenderingContext*, nsIFrame::InlineMinISizeData*, nsTextFrame::TextRunType) 	layout/generic/nsTextFrame.cpp
nsTextFrame::AddInlineMinISize(nsRenderingContext*, nsIFrame::InlineMinISizeData*) 	layout/generic/nsTextFrame.cpp
nsBlockFrame::GetMinISize(nsRenderingContext*) 	layout/generic/nsBlockFrame.cpp
nsContainerFrame::ComputeAutoSize(nsRenderingContext*, mozilla::WritingMode, mozilla::LogicalSize const&, int, mozilla::LogicalSize const&, mozilla::LogicalSize const&, mozilla::LogicalSize const&, bool) 	layout/generic/nsFrame.cpp
nsFrame::ComputeSize(nsRenderingContext*, mozilla::WritingMode, mozilla::LogicalSize const&, int, mozilla::LogicalSize const&, mozilla::LogicalSize const&, mozilla::LogicalSize const&, nsIFrame::ComputeSizeFlags) 	layout/generic/nsFrame.cpp
nsHTMLReflowState::InitAbsoluteConstraints(nsPresContext*, nsHTMLReflowState const*, mozilla::LogicalSize const&, nsIAtom*) 	layout/generic/nsHTMLReflowState.cpp
nsHTMLReflowState::InitConstraints(nsPresContext*, mozilla::LogicalSize const&, nsMargin const*, nsMargin const*, nsIAtom*) 	layout/generic/nsHTMLReflowState.cpp
nsHTMLReflowState::Init(nsPresContext*, mozilla::LogicalSize const*, nsMargin const*, nsMargin const*) 	layout/generic/nsHTMLReflowState.cpp
nsAbsoluteContainingBlock::ReflowAbsoluteFrame(nsIFrame*, nsPresContext*, nsHTMLReflowState const&, nsRect const&, bool, nsIFrame*, unsigned int&, nsOverflowAreas*) 	layout/generic/nsAbsoluteContainingBlock.cpp
nsAbsoluteContainingBlock::Reflow(nsContainerFrame*, nsPresContext*, nsHTMLReflowState const&, unsigned int&, nsRect const&, bool, bool, bool, nsOverflowAreas*) 	layout/generic/nsAbsoluteContainingBlock.cpp
nsFrame::ReflowAbsoluteFrames(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&, bool) 	layout/generic/nsFrame.cpp
nsFrame::FinishReflowWithAbsoluteFrames(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&, bool) 	layout/generic/nsFrame.cpp
nsCanvasFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) 	layout/generic/nsCanvasFrame.cpp
nsHTMLScrollFrame::ReflowScrolledFrame(ScrollReflowState*, bool, bool, nsHTMLReflowMetrics*, bool) 	layout/generic/nsContainerFrame.cpp
nsHTMLScrollFrame::ReflowContents(ScrollReflowState*, nsHTMLReflowMetrics const&) 	layout/generic/nsGfxScrollFrame.cpp
nsHTMLScrollFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) 	layout/generic/nsGfxScrollFrame.cpp
nsContainerFrame::ReflowChild(nsIFrame*, nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, int, int, unsigned int, unsigned int&, nsOverflowContinuationTracker*) 	layout/generic/nsContainerFrame.cpp
ViewportFrame::Reflow(nsPresContext*, nsHTMLReflowMetrics&, nsHTMLReflowState const&, unsigned int&) 	layout/generic/nsViewportFrame.cpp
PresShell::DoReflow(nsIFrame*, bool) 	layout/base/nsPresShell.cpp
PresShell::ProcessReflowCommands(bool) 	layout/base/nsPresShell.cpp
PresShell::FlushPendingNotifications(mozilla::ChangesToFlush) 	layout/base/nsPresShell.cpp
nsRefreshDriver::Tick(long long, mozilla::TimeStamp) 	layout/base/nsRefreshDriver.cpp
mozilla::RefreshDriverTimer::Tick(long long, mozilla::TimeStamp) 	layout/base/nsRefreshDriver.cpp
mozilla::VsyncRefreshDriverTimer::RefreshDriverVsyncObserver::NotifyVsync(mozilla::TimeStamp) 	layout/base/nsRefreshDriver.cpp
mozilla::layout::VsyncChild::RecvNotify(mozilla::TimeStamp const&) 	layout/ipc/VsyncChild.cpp
mozilla::layout::PVsyncChild::OnMessageReceived(IPC::Message const&) 	obj-firefox/x86_64/ipc/ipdl/PVsyncChild.cpp
mozilla::ipc::PBackgroundChild::OnMessageReceived(IPC::Message const&) 	obj-firefox/x86_64/ipc/ipdl/PBackgroundChild.cpp
mozilla::ipc::MessageChannel::DispatchAsyncMessage(IPC::Message const&) 	ipc/glue/MessageChannel.cpp
mozilla::ipc::MessageChannel::DispatchMessage(IPC::Message const&) 	ipc/glue/MessageChannel.cpp
mozilla::ipc::MessageChannel::OnMaybeDequeueOne() 	ipc/glue/MessageChannel.cpp
MessageLoop::DeferOrRunPendingTask(MessageLoop::PendingTask const&) 	ipc/chromium/src/base/message_loop.cc
MessageLoop::DoWork() 	ipc/chromium/src/base/message_loop.cc
mozilla::ipc::DoWorkRunnable::Run() 	ipc/glue/MessagePump.cpp
nsThread::ProcessNextEvent(bool, bool*) 	xpcom/threads/nsThread.cpp
NS_ProcessNextEvent(nsIThread*, bool) 	xpcom/glue/nsThreadUtils.cpp
nsThread::Shutdown() 	xpcom/threads/nsThread.cpp
gfxFontInfoLoader::CancelLoader() 	gfx/thebes/gfxFontInfoLoader.cpp
gfxPlatformFontList::InitFontList() 	gfx/thebes/gfxPlatformFontList.cpp
gfxMacPlatformFontList::InitFontList() 	gfx/thebes/gfxMacPlatformFontList.mm
gfxPlatformFontList::UpdateFontList() 	gfx/thebes/gfxPlatformFontList.cpp
gfxMacPlatformFontList::RegisteredFontsChangedNotificationCallback(__CFNotificationCenter*, void*, __CFString const*, void const*, __CFDictionary const*) 	gfx/thebes/gfxMacPlatformFontList.mm

Comment 5

[John Daggett (:jtd)]

Simple way to reproduce this bug:

With Firefox running, add a large folder of fonts (I used Adobe Font Folio with 2400 fonts) to ~/Library/Fonts. Doing this crashed both instances of Firefox I had running.

To reproduce the crash again, add/delete a single new font to ~/Library/Fonts. The large number of fonts makes the font loader thread run longer. Adding/deleting the font will trigger an update.

The patch I have on 1189158 will fix this.

Comment 6

[John Daggett (:jtd)]

Not seeing any OSX crashes for builds after the patch on bug 1189158 landed. I suspect for OSX that will resolve the issue here.

Comment 7

[John Daggett (:jtd)]

Marking this as resolved by bug 1189158. No OSX crashes on any of the branches after the patch for that bug landed.

Comment 8

[Wayne Mery (:wsmwk)]

correcting thunderbird tracking flag.  For thunderbird 38.2.0 it's a Mac topcrash

Comment 9

[Kent James (:rkent)]

So I assume this is now a request to uplift bug 1189158 to THUNDERBIRD_38_VERBRANCH

Comment 10

[Kent James (:rkent)]

bug 1189158 landed in FF 41 so this is not needed in the current TB beta.

Comment 11

[Kent James (:rkent)]

Too many changes between esr38 and mozilla-(gecko41) to land this with my limited knowledge of what the underlying issues are. Unblocking from comm-esr38 for TB 38.3.0.