possible new regression on the nightly channel. currently #8 top crash there. not sure about the component here, probably needs to change to something better. https://crash-stats.mozilla.com/report/index/7b0b6db1-4f18-47ac-88e4-baaf22110417 Frame Module Signature [Expand] Source 0 XUL nsUserFontSet::ReplaceFontEntry h:105 1 XUL gfxUserFontSet::OnLoadComplete gfx/thebes/gfxUserFontSet.cpp:564 2 XUL nsFontFaceLoader::OnStreamComplete layout/style/nsFontFaceLoader.cpp:228 3 XUL nsStreamLoader::OnStopRequest netwerk/base/src/nsStreamLoader.cpp:125 4 XUL nsStreamListenerTee::OnStopRequest netwerk/base/src/nsStreamListenerTee.cpp:71 5 XUL nsHttpChannel::OnStopRequest netwerk/protocol/http/nsHttpChannel.cpp:4116 6 XUL nsInputStreamPump::OnStateStop netwerk/base/src/nsInputStreamPump.cpp:578 7 XUL nsInputStreamPump::OnInputStreamReady netwerk/base/src/nsInputStreamPump.cpp:403 8 XUL nsInputStreamReadyEvent::Run xpcom/io/nsStreamUtils.cpp:114 9 XUL nsThread::ProcessNextEvent xpcom/threads/nsThread.cpp:618 10 XUL NS_ProcessPendingEvents_P nsThreadUtils.cpp:200 11 XUL nsBaseAppShell::NativeEventCallback widget/src/xpwidgets/nsBaseAppShell.cpp:130 12 XUL nsAppShell::ProcessGeckoEvents widget/src/cocoa/nsAppShell.mm:399 13 CoreFoundation __CFRunLoopDoSources0 14 CoreFoundation __CFRunLoopRun 15 CoreFoundation CFRunLoopRunSpecific 16 HIToolbox HIToolbox@0x2e7ed 17 HIToolbox HIToolbox@0x2e550 18 HIToolbox HIToolbox@0x2e4ab 19 AppKit _DPSNextEvent 20 AppKit -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] 21 AppKit -[NSApplication run] 22 XUL nsAppShell::Run widget/src/cocoa/nsAppShell.mm:746 23 XUL nsAppStartup::Run toolkit/components/startup/nsAppStartup.cpp:218 24 XUL XRE_main toolkit/xre/nsAppRunner.cpp:3754 25 firefox-bin main browser/app/nsBrowserApp.cpp:158 26 firefox-bin firefox-bin@0x953 more reports at: https://crash-stats.mozilla.com/report/list?signature=nsUserFontSet::ReplaceFontEntry users talk mostly of visiting AMO and this comment: I've been loading a page with webfonts. It crashed before fonts have been loaded. some of the code near the top of the stack changed recently. 0138798a072a 2011-04-12 11:53 +0100 Jonathan Kew - bug 633299 - don't discard font entries for @font-face rules that haven't changed. r=dbaron crashes started appearing just after that in builds from the 13th Crash count for 20110411-crashdata.csv 0 Crash count for 20110412-crashdata.csv 0 Crash count for 20110413-crashdata.csv 2 \N fx_bld:6.0a1 20110413090915 comment:\N http://c8l.ca/14y fx_bld:6.0a1 20110413090934 comment:\N Crash count for 20110414-crashdata.csv 4 \N fx_bld:6.0a1 20110414030535 comment:Loading addons.mozilla.org \N fx_bld:6.0a1 20110414030535 comment:\N http://slides.html5rocks.com/#slide-orientation fx_bld:6.0a1 20110414030535 comment:\N http://www.20minutos.es/ fx_bld:6.0a1 20110413090934 comment:\N Crash count for 20110415-crashdata.csv 4 http://blog.mozilla.com/blog/2011/02/08/mozilla-firefox-4-beta-now-including-do-not-track-capabilities/ fx_bld:6.0a1 20110413090915 comment:\N http://whiteappleer.tw/2011/04/15/mac-app-store-the-unarchiver-app/ fx_bld:6.0a1 20110414030535 comment:\N https://duckduckgo.com/?q=planet+xml+xsl+animation fx_bld:6.0a1 20110414030535 comment:\N https://mozillademos.org/demos/planetarium/demo.html fx_bld:6.0a1 20110414030535 comment:\N Crash count for 20110416-crashdata.csv 17 http://blog.mozilla.com/ fx_bld:6.0a1 20110415030523 comment:\N http://falkvinge.net/2011/04/14/european-court-of-justice-to-outlaw-internet-filtering-esp-for-copyright-enforcement/ fx_bld:6.0a1 20110415030523 comment:\N http://feastofvsenses.com/piwik/index.php?module=CoreHome&action=index&idSite=1&period=day&date=yesterday#module=Dashboard&action=embeddedIndex&idSite=1&period=day&date=yesterday fx_bld:6.0a1 20110415030523 comment:\N http://listen.grooveshark.com/#/ fx_bld:6.0a1 20110415030523 comment:\N http://localhost:3000/ fx_bld:6.0a1 20110416030532 comment:\N http://localhost:3000/archive fx_bld:6.0a1 20110415030523 comment:I've been loading a page with webfonts. It crashed before fonts have been loaded. http://localhost:3000/archive fx_bld:6.0a1 20110415030523 comment:\N http://localhost:3000/archive fx_bld:6.0a1 20110415030523 comment:\N http://localhost:3000/archive fx_bld:6.0a1 20110416030532 comment:\N http://mzl.la/hBFNBw fx_bld:6.0a1 20110416030532 comment:\N http://ourphotos.com:8888/wp/?p=16 fx_bld:6.0a1 20110415030523 comment:\N http://vojtol.pl/#home-glowna fx_bld:6.0a1 20110416030532 comment:\N http://vojtol.pl/szkocja-rozne#107 fx_bld:6.0a1 20110415030523 comment:\N http://www.bbc.co.uk/iplayer/console/b00zzqdz fx_bld:6.0a1 20110415030523 comment:\N http://www.facebook.com/extern/login_status.php? api_key=111132365592157& app_id=111132365592157& channel_url=http static.ak.fbcdn.net http%253A%252F%252Flisten.grooveshark.com%2 fx_bld:6.0a1 20110415030523 comment:\N http://www.pointlessone.org/ fx_bld:6.0a1 20110415030523 comment:\N https://addons.mozilla.org/fr/firefox/search/?sort=updated&cat=all fx_bld:6.0a1 20110415030523 comment:\N

Probably a regression from bug 633299; I'll look into it. Any reliable STR would be most helpful.

looks like there are these 4 forms of the stack. ....Signature number: 14-nsUserFontSet::ReplaceFontEntry https://bugzilla.mozilla.org/buglist.cgi?bug_id=650639 ______ distribution of 11 different stacks, looking at top 10 frames 5 stacks like 0|0|XUL|nsUserFontSet::ReplaceFontEntry 0|1|XUL|gfxUserFontSet::OnLoadComplete 0|2|XUL|nsFontFaceLoader::OnStreamComplete 0|3|XUL|nsStreamLoader::OnStopRequest 0|4|XUL|NS_InvokeByIndex_P 0|5|XUL|XPCWrappedNative::CallMethod 0|6|XUL|XPC_WN_CallMethod 0|7|XUL|js::Interpret 0|8|XUL|js::RunScript 0|9|XUL|js::Invoke 3 stacks like 0|0|XUL|nsUserFontSet::ReplaceFontEntry 0|1|XUL|gfxUserFontSet::OnLoadComplete 0|2|XUL|nsFontFaceLoader::OnStreamComplete 0|3|XUL|nsStreamLoader::OnStopRequest 0|4|XUL|nsStreamListenerTee::OnStopRequest 0|5|XUL|nsHttpChannel::OnStopRequest 0|6|XUL|nsInputStreamPump::OnStateStop 0|7|XUL|nsInputStreamPump::OnInputStreamReady 0|8|XUL|nsInputStreamReadyEvent::Run 0|9|XUL|nsThread::ProcessNextEvent 2 stacks like 0|0|XUL|nsUserFontSet::ReplaceFontEntry 0|1|XUL|gfxUserFontSet::OnLoadComplete 0|2|XUL|nsFontFaceLoader::OnStreamComplete 0|3|XUL|nsStreamLoader::OnStopRequest 0|4|XUL|nsStreamListenerTee::OnStopRequest 0|5|XUL|nsStreamListenerTee::OnStopRequest 0|6|XUL|nsHttpChannel::OnStopRequest 0|7|XUL|nsInputStreamPump::OnStateStop 0|8|XUL|nsInputStreamPump::OnInputStreamReady 0|9|XUL|nsInputStreamReadyEvent::Run 1 stacks like 0|0|libxul.so|nsUserFontSet::ReplaceFontEntry 0|1|libxul.so|gfxUserFontSet::OnLoadComplete 0|2|libxul.so|nsFontFaceLoader::OnStreamComplete 0|3|libxul.so|nsStreamLoader::OnStopRequest 0|4|libxul.so|nsCORSListenerProxy::OnStopRequest 0|5|libxul.so|nsStreamListenerWrapper::OnStopRequest 0|6|libxul.so| 0|7|libxul.so|XPCWrappedNative::CallMethod 0|8|libxul.so|XPC_WN_CallMethod 0|9|libxul.so|js::Interpret

http://www.exotissimo.com/travel/vietnam/tours/ In automation I got: Operating system: Windows NT 5.1.2600 Service Pack 3 CPU: x86 GenuineIntel family 6 model 44 stepping 2 1 CPU Crash reason: EXCEPTION_ACCESS_VIOLATION_READ Crash address: 0xffffffff82940030 Thread 0 (crashed) 0 xul.dll!nsTArray_base<nsTArrayDefaultAllocator>::Length() [nsTArray.h : 139 + 0x5] eip = 0x100de73c esp = 0x0012ce90 ebp = 0x0012ce94 ebx = 0x00000001 esi = 0x01990068 edi = 0x00000000 eax = 0x045c8068 ecx = 0x82940030 edx = 0x0358c368 efl = 0x00210202 Found by: given as instruction pointer in context 1 xul.dll!gfxMixedFontFamily::ReplaceFontEntry(gfxFontEntry *,gfxFontEntry *) [gfxUserFontSet.h : 102 + 0xa] eip = 0x10602144 esp = 0x0012ce9c ebp = 0x0012ceac Found by: call frame info 2 xul.dll!nsUserFontSet::ReplaceFontEntry(gfxProxyFontEntry *,gfxFontEntry *) [nsFontFaceLoader.cpp : 686 + 0x16] eip = 0x106020e0 esp = 0x0012ceb4 ebp = 0x0012cec4 Found by: call frame info 3 xul.dll!gfxUserFontSet::OnLoadComplete(gfxFontEntry *,unsigned char const *,unsigned int,unsigned int) [gfxUserFontSet.cpp : 564 + 0x1a] eip = 0x119f09b1 esp = 0x0012cecc ebp = 0x0012d550 Found by: call frame info locally I got: > xul.dll!nsRefPtr<gfxFontEntry>::get() Line 1098 + 0x3 bytes C++ xul.dll!nsRefPtr<gfxFontEntry>::operator gfxFontEntry *() Line 1112 C++ xul.dll!gfxMixedFontFamily::ReplaceFontEntry(gfxFontEntry * aOldFontEntry=0x079838e0, gfxFontEntry * aNewFontEntry=0x045922b8) Line 104 + 0x16 bytes C++ xul.dll!nsUserFontSet::ReplaceFontEntry(gfxProxyFontEntry * aProxy=0x079838e0, gfxFontEntry * aFontEntry=0x045922b8) Line 687 C++ xul.dll!gfxUserFontSet::OnLoadComplete(gfxFontEntry * aFontToLoad=0x079838e0, const unsigned char * aFontData=0x00000000, unsigned int aLength=23608, unsigned int aDownloadStatus=0) Line 565 C++ xul.dll!nsFontFaceLoader::OnStreamComplete(nsIStreamLoader * aLoader=0x0566b958, nsISupports * aContext=0x00000000, unsigned int aStatus=0, unsigned int aStringLen=23608, const unsigned char * aString=0x05237cb0) Line 228 + 0x20 bytes C++

I tried beating on some webfonts examples but have not yet managed to reproduce this (and simple inspection of the code didn't lead to an obvious cause). If anyone has an example that triggers this with any consistency, I'd love to hear about it.

http://www.exotissimo.com/travel/vietnam/tours/ reproduces for me on winxp and mc. I have it in vc debugger atm if you need a remote debugging bot.

Reproduces for me using Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:6.0a1) Gecko/20110426 Firefox/6.0a1 as well.

I finally reproduced this (it's sensitive to the speed/timing of font downloads), and should have a patch up shortly. Confirmed it is a regression from bug 633299; it occurs because it's possible for a font entry to get removed from the font set (and from its parent family) while a download is in progress; then when the download completes and tries to update the relevant family, it's not there. This didn't happen prior to bug 633299 because we used to completely destroy the user font set (and in the process, cancel all its downloaders) whenever the @font-face rules change.

In intensive testing on the http://www.exotissimo.com/ site mentioned above, I can no longer trigger a crash with this patch. (It was sporadic for me previously, depending on the timing of stylesheet and font downloads, I believe.)

Better version of the patch - functionally equivalent to the original, but it's cleaner to reset the proxy's loading state from nsFontFaceLoader::Cancel(). Also eliminates more of the static_cast<gfxProxyFontEntry*> usage from the code; an nsFontFaceLoader is always created referring to a proxy entry, so we can simply declare it as such rather than checking its mIsProxy field and casting in various places. And letting the compiler do better type-checking makes for less error-prone code going forward. :)

The only interesting bits seem to be the changes to nsFontFaceLoader::Cancel() and nsUserFontSet::UpdateRules(); everything else seems to be the type changes, which look good. As far as the change in UpdateRules -- does this cause us to stop loads for fonts that we still want to continue loading because they'll still be in the font set? Or were those loads not actually useful anyway? And in the change to Cancel() -- what's the lifetime of these proxy entries? Do they outlast the particular attempt to load?

(In reply to comment #11) > As far as the change in UpdateRules -- does this cause us to stop loads for > fonts that we still want to continue loading because they'll still be in the > font set? Or were those loads not actually useful anyway? Yes, I think it could cancel loads that we end up re-doing, and in this case it's not optimal to cancel them (although we'll still be in much better shape than we were prior to bug 633299, as we don't discard fonts that have actually been loaded). But I'm a bit nervous of trying to be smarter about it to try and preserve these, and potentially leaving corner cases that still break (and that are hard to test/verify as they're so timing-sensitive). I'm more comfortable doing the simple fix for this crash, and then if we want to optimize behavior further, do that at leisure in a followup. > And in the change to Cancel() -- what's the lifetime of these proxy entries? > Do they outlast the particular attempt to load? Yes, in the case where the rule (and hence proxy entry) survives the font-set update. The proxy works its way through its list of sources, and if we cancel a loader, we want it to retry that same source next time rather than move on to the next (as we'd do when calling LoadNext on a proxy that is already in the loading state). Resetting the state to not-loading means it will start again at the current position in its source list.

Comment on attachment 528619 [details] [diff] [review] patch, ensure loaders are cancelled when updating the font set - improved ok, sounds good, r=dbaron

Landed on mozilla-central: http://hg.mozilla.org/mozilla-central/rev/a9dd6038db6a I'm expecting this to resolve these crashes, but will leave the bug open for now until we see crash-stats from the next Nightly build.

Crash-stats shows no crashes in builds later than 20110427, for either this signature or the Windows version of the crash, shown in bug 651180; accordingly, marking this as fixed.

please nominate for mozilla-beta.

Jonathan: I see a handful of crashes on current Mac trunk builds - http://tinyurl.com/68wuac4. Should I file a new bug for this?

Comment on attachment 528619 [details] [diff] [review] patch, ensure loaders are cancelled when updating the font set - improved Nominating for mozilla-beta as per comment #16; this fixes a potential crash. Assuming we take this patch, we _must_ also take Bug 653977 to avoid the disappearing-text regression described there.

(In reply to comment #17) > Jonathan: I see a handful of crashes on current Mac trunk builds - > http://tinyurl.com/68wuac4. Should I file a new bug for this? Yes, please do, and cc me; I'll try to investigate what could be happening there.

(In reply to comment #18) > Nominating for mozilla-beta as per comment #16; this fixes a potential > crash. Assuming we take this patch, we _must_ also take Bug 653977 to avoid > the disappearing-text regression described there. Hmm, on looking again, I don't think this is applicable to m-b at all. Bug 633299 landed on trunk _after_ the merge to Aurora on 4/11, and therefore none of this is present in m-a / m-b.

Comment on attachment 528619 [details] [diff] [review] patch, ensure loaders are cancelled when updating the font set - improved unsetting the approval request as this doesn't affect Aurora/Beta