Closed
Bug 462593
Opened 16 years ago
Closed 16 years ago
Crash [out of memory] when rapidly opening multiple frames with downloadable fonts
Categories
(Core :: Layout, defect, P2)
Tracking
()
VERIFIED
FIXED
People
(Reporter: martijn.martijn, Assigned: roc)
References
()
Details
(Keywords: crash, testcase, verified1.9.1)
Attachments
(5 files)
(deleted),
text/plain; charset=shift-jis
|
Details | |
(deleted),
application/java-archive
|
Details | |
(deleted),
patch
|
benjamin
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
jtd
:
review+
|
Details | Diff | Splinter Review |
(deleted),
patch
|
Details | Diff | Splinter Review |
Might be related to bug 460037. Steps to reproduce: Visit url, let it open for at least 30 seconds or so, then close the tab of that page. Result: crash http://crash-stats.mozilla.com/report/index/1e7a3da1-a799-11dd-a97d-001a4bd43e5c?p=1 0 xul.dll gfxMixedFontFamily::RemoveFontEntry obj-firefox/dist/include/thebes/gfxUserFontSet.h:113 1 xul.dll gfxUserFontSet::LoadNext gfx/thebes/src/gfxUserFontSet.cpp:371 2 xul.dll gfxUserFontSet::OnLoadComplete gfx/thebes/src/gfxUserFontSet.cpp:270 3 xul.dll nsFontFaceLoader::OnStreamComplete layout/style/nsFontFaceLoader.cpp:142
Reporter | ||
Updated•16 years ago
|
Summary: Crash [@ gfxMixedFontFamily::RemoveFontEntry] when closing tab in this → Crash [@ gfxMixedFontFamily::RemoveFontEntry] when closing tab in this case
Reporter | ||
Updated•16 years ago
|
Flags: blocking1.9.1?
Assignee | ||
Updated•16 years ago
|
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2
Updated•16 years ago
|
Status: NEW → RESOLVED
Closed: 16 years ago
Resolution: --- → DUPLICATE
Comment 2•16 years ago
|
||
Reopening. With 10 tabs open, the original crash in user font set code doesn't occur but an out-of-memory condition occurs. Need to confirm the underlying cause of this.
Comment 3•16 years ago
|
||
Updated•16 years ago
|
Attachment #352059 -
Attachment mime type: text/plain → text/plain; charset=shift-jis
Updated•16 years ago
|
Summary: Crash [@ gfxMixedFontFamily::RemoveFontEntry] when closing tab in this case → Crash [out of memory] when rapidly opening multiple frames with downloadable fonts
Comment 5•16 years ago
|
||
Stack crawl snippet: msvcr80d.dll!_onexit_nolock msvcr80d.dll!_CxxThrowException msvcr80d.dll!operator new gklayout.dll!nsIFrame::GetOverflowAreaProperty gklayout.dll!nsIFrame::FinishAndStoreOverflow gklayout.dll!nsIFrame::FinishAndStoreOverflow gklayout.dll!nsBlockFrame::Reflow gklayout.dll!nsContainerFrame::ReflowChild gklayout.dll!CanvasFrame::Reflow gklayout.dll!nsContainerFrame::ReflowChild gklayout.dll!nsHTMLScrollFrame::ReflowScrolledFrame gklayout.dll!nsHTMLScrollFrame::ReflowContents gklayout.dll!nsHTMLScrollFrame::Reflow
Comment 6•16 years ago
|
||
That first stack has some weird recursion going on.... What happens if you quit the browser before hitting OOM? Are there leaks in this case?
that's probably not recursion. mw, when you see nsThread::Shutdown in stacks, can you attach the stacks for the other threads too? (also for bonus points, can you check the nsThread objects and try to pair nsThreads to system threads)
Assignee | ||
Updated•16 years ago
|
Attachment #353027 -
Attachment mime type: application/zip → application/java-archive
Assignee | ||
Comment 8•16 years ago
|
||
The URL for the zipped-up testcase is jar:https://bugzilla.mozilla.org/attachment.cgi?id=353027!/crash1.htm
Assignee | ||
Comment 9•16 years ago
|
||
With Martin's stack, I get this #6 0x0309b509 in gfxQuartzFontCache::MakePlatformFont (this=0x48b98d0, aProxyEntry=0xebae3c0, aFontData=0x7200008 "", aLength=58736) at /Users/roc/mozilla-trunk/xpcom/string/src/nsReadableUtils.cpp:812 1322 NS_ConvertUTF16toUTF8(proxyEntry->mFamily->Name()).get()); (gdb) p proxyEntry->mFamily[0] warning: can't find linker symbol for virtual table for `gfxMixedFontFamily' value $9 = { <gfxFontFamily> = { _vptr$gfxFontFamily = 0xda264e0, mRefCnt = { mValue = 23726592 }, mName = { <nsAString_internal> = { mData = 0xeb63470, mLength = 246821328, mFlags = 6512941 }, <No data fields>} }, proxyEntry looks OK but proxyEntry->mFamily is obviously garbage.
Assignee | ||
Comment 10•16 years ago
|
||
Interestingly this is after we failed to validate the font: WARNING: invalid font (bad checksum): file /Users/roc/mozilla-trunk/gfx/thebes/src/gfxFontUtils.cpp, line 768 WARNING: downloaded font error, invalid font data for (Bitstream Vera Serif Bold): file /Users/roc/mozilla-trunk/gfx/thebes/src/gfxQuartzFontCache.mm, line 1323 WARNING: invalid font (bad checksum): file /Users/roc/mozilla-trunk/gfx/thebes/src/gfxFontUtils.cpp, line 768
Assignee | ||
Comment 11•16 years ago
|
||
OK, here's what happens. We see the first @font-face rule and kick off a load of "Bitstream Vera Serif Bold" with a certain gfxProxyEntry and gfxMixedFontFamily. Then something happens (seeing the second @font-face rule?) and we do FlushUserFontSet. We reprocess the first @font-face rule and kick off another load of "Bitstream Vera Serif Bold" with different gfxProxyEntry and gfxMixedFontFamily objects. We get an nsFontFaceLoader::OnStreamComplete for the first load. The font's invalid, so all srcs for that face failed, so all faces for that family object failed, so gfxUserFontSet::LoadNext does "RemoveFamily(family->Name());". Unfortunately this deletes the second gfxMixedFontFamily object since we're in a new gfxUserFontSet now! So the second gfxProxyEntry has a dangling pointer.
Assignee | ||
Comment 12•16 years ago
|
||
For the real patch, I need nsPtrHashKey. So I'm adding it to nsHashKeys.
Assignee: nobody → roc
Attachment #355737 -
Flags: review?(benjamin)
Assignee | ||
Comment 13•16 years ago
|
||
-- Keep a list of active nsFontFaceLoaders in the nsUserFontSet -- Let each nsFontFaceLoader have a strong reference to its nsUserFontSet -- When the nsUserFontSet gets detached from the presentation, it cancels all its nsFontFaceLoaders, which cancels the download and unhooks the reference from the loader to the fontset This fixes the bug here by ensuring that a loader can never affect a font set that's not the active font set for the presentation.
Attachment #355739 -
Flags: review?(jdaggett)
Assignee | ||
Updated•16 years ago
|
Whiteboard: [needs review]
Updated•16 years ago
|
Attachment #355737 -
Flags: review?(benjamin) → review+
Comment 15•16 years ago
|
||
The part2 patch needed to be adjusted because of some code I checked in a couple days ago. It also failed with the original testcase because in some cases ctx->PresShell might be null. I added and null check and it no longer fails with the original testcase but I still see an assertion about the null presShell ptr. Original testcase is no longer available at the download site so I uploaded it here: http://people.mozilla.org/~jdaggett/bugs/462593iframefontloader.zip
Updated•16 years ago
|
Attachment #355739 -
Flags: review?(jdaggett) → review+
Comment 16•16 years ago
|
||
Comment on attachment 355739 [details] [diff] [review] part 2: real fix Looks good but not sure whether the assertion with the original testcase is valid or not.
Assignee | ||
Comment 17•16 years ago
|
||
I think we need to call nsUserFontSet::Destroy from nsPresContext::SetShell(nsnull). That should fix it.
Assignee | ||
Updated•16 years ago
|
Whiteboard: [needs review] → [needs landing]
Assignee | ||
Comment 18•16 years ago
|
||
Pushed b2c58375f633 with that change.
Status: REOPENED → RESOLVED
Closed: 16 years ago → 16 years ago
Resolution: --- → FIXED
Whiteboard: [needs landing]
Assignee | ||
Updated•16 years ago
|
Whiteboard: [needs 191 landing]
Comment 19•16 years ago
|
||
Patches pushed to 1.9.1 branch on behalf of roc. Part 1: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/5954c9f70316 Part 2: http://hg.mozilla.org/releases/mozilla-1.9.1/rev/36d04c3ebb98
Keywords: fixed1.9.1
Whiteboard: [needs 191 landing]
Reporter | ||
Comment 20•16 years ago
|
||
Verified fixed, using: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.2a1pre) Gecko/20090407 Minefield/3.6a1pre (.NET CLR 3.5.30729)
Status: RESOLVED → VERIFIED
Comment 21•16 years ago
|
||
verified FIXED on Shiretoko: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1b4pre) Gecko/20090422 Shiretoko/3.5b4pre ID:20090422042031
Keywords: fixed1.9.1 → verified1.9.1
You need to log in
before you can comment on or make changes to this bug.
Description
•