Closed Bug 486470 Opened 16 years ago Closed 16 years ago

sporadic crash on Windows tinderboxen in gfxTextRun::~gfxTextRun()

Categories

(Core :: Graphics, defect, P2)

x86
Windows XP
defect

Tracking

()

VERIFIED FIXED
mozilla1.9.2a1

People

(Reporter: jtd, Assigned: dcamp)

References

Details

(Keywords: crash, fixed1.9.1, intermittent-failure)

Attachments

(4 files, 1 obsolete file)

Windows unit test machine is periodically crashing in gfxTextRun::~gfxTextRun() when running mochitest-plain tests. Example logs: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1238606166.1238611997.12746.gz&fulltext=1 http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1238599133.1238605868.953.gz&fulltext=1 Crash reason: EXCEPTION_ACCESS_VIOLATION Crash address: 0x0 Stack crawl: 0 xul.dll!nsTArray<gfxTextRun::GlyphRun>::~nsTArray<gfxTextRun::GlyphRun>() [nsTArray.h:7f359d93170a : 267 + 0x5] eip = 0x101c5059 esp = 0x0012e11c ebp = 0x0012e174 ebx = 0x0719c401 esi = 0x0719c428 edi = 0x0ac5841c eax = 0x00000000 ecx = 0x0719c428 edx = 0xf57b0012 efl = 0x00010202 1 xul.dll!gfxTextRun::~gfxTextRun() [gfxFont.cpp:7f359d93170a : 1409 + 0xf] eip = 0x10632267 esp = 0x0012e124 ebp = 0x0012e174 2 xul.dll!gfxTextRun::`vector deleting destructor'(unsigned int) + 0x35 eip = 0x101bdd3c esp = 0x0012e12c ebp = 0x0012e174 3 xul.dll!nsTextFrame::ClearTextRun() [nsTextFrameThebes.cpp:7f359d93170a : 3692 + 0x7] eip = 0x1027213a esp = 0x0012e138 ebp = 0x0012e174 ebx = 0x0719c418 4 xul.dll!nsTextFrame::Destroy() [nsTextFrameThebes.cpp:7f359d93170a : 3345 + 0x4] eip = 0x102729eb esp = 0x0012e144 ebp = 0x0012e174 ebx = 0x00000000 5 xul.dll!nsLineBox::DeleteLineList(nsPresContext *,nsLineList &) [nsLineBox.cpp:7f359d93170a : 337 + 0x7] eip = 0x1033d6a8 esp = 0x0012e14c ebp = 0x0012e174 6 xul.dll!nsBlockFrame::Destroy() [nsBlockFrame.cpp:7f359d93170a : 298 + 0x9] eip = 0x103376f7 esp = 0x0012e158 ebp = 0x0012e174 Going back to 3/20, I don't see the exact same crash but there are other crashes in GlyphRun code: Example: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1237610333.1237613764.10070.gz&fulltext=1 0 xul.dll!gfxTextRun::FindFirstGlyphRunContaining(unsigned int) [gfxFont.cpp:49d7fee2e9b4 : 2175 + 0x3] eip = 0x1062eefa esp = 0x0012bc90 ebp = 0x0012bcd4 ebx = 0x00000001 esi = 0x00000000 edi = 0x00000000 eax = 0x00000000 ecx = 0x0313a1f8 edx = 0x00000000 efl = 0x00010297 1 xul.dll!gfxTextRun::GlyphRunIterator::GlyphRunIterator(gfxTextRun *,unsigned int,unsigned int) [gfxFont.h:49d7fee2e9b4 : 1313 + 0xd] eip = 0x101bda30 esp = 0x0012bc9c ebp = 0x0012bcd4 2 xul.dll!gfxTextRun::MeasureText(unsigned int,unsigned int,gfxFont::BoundingBoxType,gfxContext *,gfxTextRun::PropertyProvider *) [gfxFont.cpp:49d7fee2e9b4 : 1925 + 0xe] eip = 0x1062faff esp = 0x0012bca8 ebp = 0x0012bcd4 3 xul.dll!gfxTextRun::BreakAndMeasureText(unsigned int,unsigned int,int,double,gfxTextRun::PropertyProvider *,int,double *,gfxFont::RunMetrics *,gfxFont::BoundingBoxType,gfxContext *,int *,unsigned int *,int,gfxBreakPriority *) [gfxFont.cpp:49d7fee2e9b4 : 2103 + 0x21] eip = 0x1062fefd esp = 0x0012bcdc ebp = 0x0012c418 4 xul.dll!nsTextFrame::Reflow(nsPresContext *,nsHTMLReflowMetrics &,nsHTMLReflowState const &,unsigned int &) [nsTextFrameThebes.cpp:49d7fee2e9b4 : 5976 + 0x71] eip = 0x10275bfe esp = 0x0012c45c ebp = 0x0012c5e8 ebx = 0x0b694df0 5 xul.dll!nsLineLayout::ReflowFrame(nsIFrame *,unsigned int &,nsHTMLReflowMetrics *,int &) [nsLineLayout.cpp:49d7fee2e9b4 : 852 + 0x2e] eip = 0x103a037e esp = 0x0012c654 ebp = 0x0012c72c ebx = 0x0012c840 6 xul.dll!nsBlockFrame::ReflowInlineFrame(nsBlockReflowState &,nsLineLayout &,nsLineList_iterator,nsIFrame *,LineReflowStatus *) [nsBlockFrame.cpp:49d7fee2e9b4 : 3594 + 0x13] eip = 0x10339af0 esp = 0x0012c79c ebp = 0x0012c7cc ebx = 0x0b694f1c 7 xul.dll!nsBlockFrame::DoReflowInlineFrames(nsBlockReflowState &,nsLineLayout &,nsLineList_iterator,int *,LineReflowStatus *,int) [nsBlockFrame.cpp:49d7fee2e9b4 : 3415 + 0x15] eip = 0x1033a364 esp = 0x0012c7d4 ebp = 0x0012c814 ebx = 0x0b694f1c All these crashes occur running this test: *** 31828 INFO Running /tests/dom/tests/mochitest/ajax/offline/test_xhtmlManifest.xhtml... Not sure if there are other factors that match (e.g. same tinderbox machine).
Flags: blocking1.9.1?
Browsing through the Tinderbox logs around 3/14, I don't see this crash, so most likely a change in the past couple weeks.
Whiteboard: [orange]
Possibly related to bug 484177?
Happened again: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1238684373.1238693310.29046.gz (also running tests/dom/tests/mochitest/ajax/offline/test_xhtmlManifest.xhtml)
(In reply to comment #2) > Possibly related to bug 484177? I'm guessing these are related. I went back and did an analysis of Tinderbox crashes, looking for crashes involving glyph runs and test_xhtmlManifest.xhtml. The crashes in FindFirstGlyphRunContaining started 3/18 and stopped 3/23, then the ~gfxTextRun crashes started 4/1 just after 8am. Hard to see changesets that relate to this, the only one that seems related to the end of the FindFirstGlyphRunContaining crashes is: Joe Drew — Bug 484076 - Shrink the glyph cache to fix tp_Rss regression. r=me http://hg.mozilla.org/mozilla-central/rev/2617fe64e693 Mar 23 18:54:20 I don't see any obvious code change that would induce this bug. Full history: WINNT 5.2 mozilla-central unit test on 2009/03/18 21:16:38 http://hg.mozilla.org/mozilla-central/index.cgi/rev/4e1a4c23041c gfxTextRun::FindFirstGlyphRunContaining(unsigned int) [gfxFont.cpp:4e1a4c23041c : 2175 + 0x3] WINNT 5.2 mozilla-central unit test on 2009/03/19 01:42:40 http://hg.mozilla.org/mozilla-central/index.cgi/rev/032ffaec3006 gfxTextRun::FindFirstGlyphRunContaining(unsigned int) [gfxFont.cpp:032ffaec3006 : 2172 + 0x3] WINNT 5.2 mozilla-central unit test on 2009/03/19 07:48:08 http://hg.mozilla.org/mozilla-central/index.cgi/rev/80da01543b56 gfxTextRun::FindFirstGlyphRunContaining(unsigned int) [gfxFont.cpp:80da01543b56 : 2172 + 0x3] WINNT 5.2 mozilla-central unit test on 2009/03/19 21:29:08 http://hg.mozilla.org/mozilla-central/index.cgi/rev/ac8e14d8a263 gfxTextRun::FindFirstGlyphRunContaining(unsigned int) [gfxFont.cpp:ac8e14d8a263 : 2172 + 0x3] WINNT 5.2 mozilla-central unit test on 2009/03/20 18:04:18 http://hg.mozilla.org/mozilla-central/index.cgi/rev/49d7fee2e9b4 gfxTextRun::FindFirstGlyphRunContaining(unsigned int) [gfxFont.cpp:49d7fee2e9b4 : 2175 + 0x3] WINNT 5.2 mozilla-central unit test on 2009/03/20 21:38:53 http://hg.mozilla.org/mozilla-central/index.cgi/rev/49d7fee2e9b4 gfxTextRun::FindFirstGlyphRunContaining(unsigned int) [gfxFont.cpp:49d7fee2e9b4 : 2175 + 0x3] WINNT 5.2 mozilla-central unit test on 2009/03/21 17:57:24 http://hg.mozilla.org/mozilla-central/index.cgi/rev/8d2c566f3256 gfxTextRun::FindFirstGlyphRunContaining(unsigned int) [gfxFont.cpp:8d2c566f3256 : 2175 + 0x3] WINNT 5.2 mozilla-central unit test on 2009/03/23 18:24:55 http://hg.mozilla.org/mozilla-central/index.cgi/rev/72969ace6b09 gfxTextRun::FindFirstGlyphRunContaining(unsigned int) [gfxFont.cpp:72969ace6b09 : 2172 + 0x3] WINNT 5.2 mozilla-central unit test on 2009/04/01 08:18:53 http://hg.mozilla.org/mozilla-central/index.cgi/rev/7f359d93170a nsTArray<gfxTextRun::GlyphRun>::~nsTArray<gfxTextRun::GlyphRun>() [nsTArray.h:7f359d93170a : 267 + 0x5] WINNT 5.2 mozilla-central unit test on 2009/04/01 10:16:06 http://hg.mozilla.org/mozilla-central/index.cgi/rev/479d0bfb14cb nsTArray<gfxTextRun::GlyphRun>::~nsTArray<gfxTextRun::GlyphRun>() [nsTArray.h:479d0bfb14cb : 267 + 0x5] WINNT 5.2 mozilla-central unit test on 2009/04/01 20:39:17 http://hg.mozilla.org/mozilla-central/index.cgi/rev/8d60bedd277b nsTArray<gfxTextRun::GlyphRun>::~nsTArray<gfxTextRun::GlyphRun>() [nsTArray.h:8d60bedd277b : 267 + 0x5] WINNT 5.2 mozilla-central unit test on 2009/04/02 01:01:05 http://hg.mozilla.org/mozilla-central/index.cgi/rev/cc4fae01938c nsTArray<gfxTextRun::GlyphRun>::~nsTArray<gfxTextRun::GlyphRun>() [nsTArray.h:cc4fae01938c : 267 + 0x5] WINNT 5.2 mozilla-central unit test on 2009/04/02 01:12:07 http://hg.mozilla.org/mozilla-central/index.cgi/rev/d3240cd1c610 nsTArray<gfxTextRun::GlyphRun>::~nsTArray<gfxTextRun::GlyphRun>() [nsTArray.h:d3240cd1c610 : 267 + 0x5] WINNT 5.2 mozilla-central unit test on 2009/04/02 07:59:33 http://hg.mozilla.org/mozilla-central/index.cgi/rev/ccbfc42dbaaf nsTArray<gfxTextRun::GlyphRun>::~nsTArray<gfxTextRun::GlyphRun>() [nsTArray.h:ccbfc42dbaaf : 267 + 0x5]
I ran mochitests in a home-made Windows opt build several times and failed to reproduce the crash. I did crash several times in xpcshell (httpd.js server I presume)...
I was going to try running mochitests in a nightly build but then I realized I can't do that.
It'd be very surprising if just changing the Cairo glyph cache size caused crashes. Jeff?
(In reply to comment #6) > I was going to try running mochitests in a nightly build but then I realized I > can't do that. :-( Sadly still correct. We're getting closer though, as we continue the "separate-unittest-from-build" work. See bug#383136 and bug#486783 for details.
(In reply to comment #7) > It'd be very surprising if just changing the Cairo glyph cache size caused > crashes. Jeff? The cairo glyph cache change just seems to have been checked in around the time the first flavor of glyph run associated crash *stopped* occurring. Somehow that may have perturbed the conditions under which that crash occurred, it's not necessarily the cause of either of the crashes.
I'm able to reproduce this on my WinXP box. Steps: 1. Apply attached patch to set the textrun cache expiration timeout to 10ms 2. Build with tinderbox-like mozconfig: export MOZ_DEBUG_SYMBOLS=1 . $topsrcdir/browser/config/mozconfig mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/obj-tinder ac_add_options --disable-debug ac_add_options --enable-tests ac_add_options --enable-optimize ac_add_options --enable-logrefcnt mk_add_options MOZ_CO_PROJECT=browser 3. Run dom mochitests: TEST_PATH=dom/tests/mochitest make mochitest-plain Result: 2 out of 3 times this will crash in the same place the crash occurs on the tinderbox. My guess is that there's a missing refcount increment of a text run somewhere. Tickling the textrun expiration timeout simulates the behavior running on a mud-slow tinderbox VM. This is a bit of a heisenbug, if you poke at it the problem disappears. I tried attaching a debugger while the mochitests were running and that caused the bug not to occur.
That's a great idea ... I'll give it a go
Flags: blocking1.9.1? → blocking1.9.1+
Priority: -- → P2
I tried that and couldn't reproduce over here :-(.
This patch sets the expire timeout of both the TextRunExpiringCache and the FrameTextRunCache caches to 1ms. With this setting, the crashes happen frequently with a fully-optimized build, never with a debug build. Simply running this with a debugger attached causes everything to run fine. So this patch uses logs to track what's going on: NSPR_LOG_FILE=textrun.out NSPR_LOG_MODULES=textrun:5,textframe:5 The direct cause of the crash is something is trashing the mHdr field of the mGlyphRuns array, this results in a crash when calling mGlyphRuns->Length(). 0[636a48]: textrun: destr (3fc9910) mGlyphRuns dump: 03fc9924 00000001 80000001 01e8eb08 00000000 00008aa1 0[636a48]: textrun: destr (6e78d60) mGlyphRuns dump: 06e78d74 00000001 80000001 02861890 00000000 00008a9d 0[636a48]: textrun: destr (6b8b258) mGlyphRuns dump: 06b8b26c 00000001 80000001 01e56d10 00000000 00008ab0 0[636a48]: textrun: destr (46055a8) mGlyphRuns dump: 046055bc 00000001 80000001 01e56d10 00000000 00008aae 0[636a48]: textrun: destr (403fdc8) mGlyphRuns dump: 0403fddc 00000001 80000001 01e56d10 00000000 00008aac 0[636a48]: textrun: destr (2a55758) mGlyphRuns dump: 02a5576c 00000001 80000001 01e56d10 00000000 00008aaa 0[636a48]: textrun: destr (745abf8) mGlyphRuns dump: 0745ac0c 00000001 80000001 01e56d10 00000000 00008aa9 0[636a48]: textrun: destr (5c8f5a8) mGlyphRuns dump: 05c8f5bc 00000001 80000001 01e56d10 00000000 00008aa8 0[636a48]: textrun: destr (6c8cd30) mGlyphRuns dump: 06c8cd44 00000001 80000001 01e56d10 00000000 00008aa7 0[636a48]: textrun: destr (1bb0638) mGlyphRuns dump: 00000000 00000001 80000001 01e56d10 00000000 00008aa6 The next step is to track down the code trashing this memory.
Attachment #371208 - Attachment is obsolete: true
I didn't manage to repro this yet myself, but I wonder if it is caused by bug 487063. There's a patch there you might like to try.
Try moving mGlyphRuns to the end of gfxTextRun and hex-dump the entire text run. If the mHdr field and *only* the mHdr field is still corrupted the same way, that strongly suggests the corruption is happening via a negative index through mGlyphRuns, although I can't see by looking at the code how that could happen.
I can reproduce this fairly easily now, with the expiration timers set to 1ms (as per comment 14), the crash occurs running a single testcase, the one just before test_xhtmlManifest.xhtml in the dom/tests/mochitest/ajax/offline set of tests. But the bug is *very* sensitive to memory placement, adding a single long word into gfxTextRun before or after mGlyphRuns prevents the crash. Likewise, attaching a debugger while running the test also prevents the crash. Since the crash always occurs when the first longword of mGlyphRuns is set to zero (where the array base is stored), I set up checks to figure out where this longword is getting corrupted. I bracketed all calls to gfxTextRun methods with corruption checks, and did the same for many methods in nsTextFrame, but the corruption doesn't seem to occur in any of these places, the first place it shows up is when the NotifyExpiry method fires in the FrameTextRunCache, just before the text run is deleted. There does seem to be a consistent pattern, the same text run gets corrupted each time the crash occurs, a text run with a single space. Tomorrow I'm going to try to get the exact pattern down and try attaching a debugger when the text run is constructed to see if a data breakpoint can catch the culprit.
(In reply to comment #17) > There does seem to be a consistent pattern, the same text run gets corrupted > each time the crash occurs, a text run with a single space. Hm.. dumb question, I seem to recall that we cache a single space textrun instead of creating it each time, is it that cached one that gets corrupted?
You probably don't want to hear about more of these but in case they are of some use, a couple more from today on WINNT 5.2 mozilla-central unit test - http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1239219938.1239226093.11649.gz http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1239198392.1239205132.2447.gz
We don't really have a cached single-space textrun. It would be interesting to know what's in the textrun that gets corrupted, though.
So I tracked this down to some offline cache code. Based on logging, I set up a debug break as close as possible to when the corruption occurs, then set up data breakpoints. For some reason, the first longword in the mGlyphRuns struct is getting overwritten within a call to nsOfflineCacheUpdate::Finish(). At the end of that code is: nsresult rv = NS_OK; if (mOwner) { rv = mOwner->UpdateFinished(this); mOwner = nsnull; } return rv; The optimized assembly for this is: if (mOwner) { 104529B6 mov ecx,dword ptr [esi+10h] 104529B9 xor eax,eax 104529BB test ecx,ecx 104529BD je nsOfflineCacheUpdate::Finish+144h (104529C8h) rv = mOwner->UpdateFinished(this); 104529BF mov eax,dword ptr [ecx] 104529C1 push esi 104529C2 call dword ptr [eax] mOwner = nsnull; 104529C4 and dword ptr [esi+10h],0 } After calling UpdateFinished, ESI is set to the address of a gfxTextRun, so the last instruction above smashes the longword in the mGlyphRuns struct. Guessing this somehow relates to calls to deallocated objects.
(In reply to comment #21) > if (mOwner) { > rv = mOwner->UpdateFinished(this); > mOwner = nsnull; > } Looking at nsOfflineCacheUpdateService::UpdateFinished (one of the possible things that UpdateFinished could be, I think), it seems like the UpdateFinished call may well cause the last release of its |aUpdate| parameter, which is |this| in the above code, which could make the |mOwner = nsnull| an access to freed memory. Maybe whoever's calling nsOfflineCacheUpdate::Finish ought to be holding a reference?
So the sequence of events that leads to the memory corruption involves nsOfflineCacheUpdateItem's. These contain a non-ref-counted pointer to a nsOfflineCacheUpdate object. One of these is getting destructed but it's still referenced in a nsOfflineCacheUpdateItem. When nsOfflineCacheUpdateItem::Run is called from an event, it calls the LoadCompleted method of deleted nsOfflineCacheUpdate object and bad things happen. The exact call stack is: xul.dll!nsOfflineCacheUpdate::Finish() 行 1991 C++ xul.dll!nsOfflineCacheUpdate::ProcessNextURI() 行 1601 + 0xb バイト C++ xul.dll!nsOfflineCacheUpdate::LoadCompleted() 行 1453 C++ > xul.dll!nsOfflineCacheUpdateItem::Run() 行 477 C++ xul.dll!nsThread::ProcessNextEvent(int mayWait=0x00000001, int * result=0x0012fc70) 行 511 C++ xul.dll!NS_ProcessNextEvent_P(nsIThread * thread=0x00000001, int mayWait=0x00000001) 行 230 + 0xd バイト C++ (行) == line, (バイト) == bytes I built with the mozconfig described in comment 10, running with the attached debug patch. The crash occurs with a single testcase: TEST_PATH=dom/tests/mochitest/ajax/offline/test_updatingManifest.html make mochitest-plain The debug patch adds logging: NSPR_LOG_FILE=textrun.out NSPR_LOG_MODULES=textrun:5,textframe:5,nsOfflineCacheUpdate:5 There are a couple DebugBreak's, the first just before the corruption is about to occur and the second when the corruption has just occurred.
Attached file sample log file (deleted) —
For this run, the corruption occur in the text run at 0x03f369c8 0[636a60]: nsOfflineCacheUpdate::NotifyStarted [3f369c8, 2d93058] 0[636a60]: nsOfflineCacheUpdate::LoadCompleted [3f369c8] 0[636a60]: nsOfflineCacheUpdate::NotifyCompleted [3f369c8, 2d93058] 0[636a60]: nsOfflineCacheUpdate::ProcessNextURI [3f369c8, current=1, numItems=1] 0[636a60]: nsOfflineCacheUpdate::Finish begin [3f369c8 thread: 000007c4] 0[636a60]: nsOfflineCacheUpdate::NotifyNoUpdate [3f1a680] 0[636a60]: nsOfflineCacheUpdate destr [3f369c8 thread:000007c4] 0[636a60]: textrun: const (3f369c8) count: 249, ptr: 0x03f369d8 thread: 000007c4 * 0[636a60]: textrun: const (3f369c8) - [1] (20) 0[636a60]: textrun (3f369c8) AddGlyphRun 1e4d938 0 numRuns: 0 0[636a60]: textrun (3f369c8) AddGlyphRun append 1e4d938 0 0[636a60]: nsOfflineCacheUpdate::Finish begin [3f1a680 thread: 000007c4] 0[636a60]: nsOfflineCacheUpdateService::UpdateFinished [294cf70, update=3f1a680] 0[636a60]: nsOfflineCacheUpdate::RemoveObserver [3f1a680] 0[636a60]: nsOfflineCacheUpdate::RemoveObserver [3f1a680] 0[636a60]: nsOfflineCacheUpdateService::ProcessNextUpdate [294cf70, num=0] 0[636a60]: nsOfflineCacheUpdate::Finish end [3f1a680 thread:000007c4] 0[636a60]: nsOfflineCacheUpdate::Finish end [3f369c8 thread:000007c4] 0[636a60]: textrun (3f369c8) mGlyphRuns corrupt - bad textrun check So the textrun is using memory previously used for a nsOfflineCacheUpdate object. The "Finish end [3f369c8" shows part of the call stack executed *after* the destrcution of the nsOfflineCacheUpdate object. Guessing this was caused by one of the recent updates to the offsite cache code: http://hg.mozilla.org/mozilla-central/log/907dbdbd084f/uriloader/prefetch/nsOfflineCacheUpdate.cpp
Assignee: roc → nobody
Assignee: nobody → dcamp
The call stack at the time of this stuff would probably be useful in terms of figuring out who should be holding a reference (but isn't).
The log is enough - Finish() takes a ref and then schedules an unref on the main loop. The idea was to avoid having kungFuDeathGrips for every potential caller of Finish(). But one of the UpdateFinished() implementations eventually fires an event to content, and one of the test handlers spins and event loop with a sync xhr. So we ought to just use kungFuDeathGrips, I'll put together a patch today.
Attached patch fix (deleted) — Splinter Review
Attachment #372148 - Flags: superreview?(jst)
Attachment #372148 - Flags: review?(honzab.moz)
Attachment #372148 - Attachment is patch: true
Attachment #372148 - Attachment mime type: application/octet-stream → text/plain
Comment on attachment 372148 [details] [diff] [review] fix I would say that only place we need the grip is nsOfflineCacheUpdate::ManifestCheckCompleted, however better be safer. Then, why are you using nsCOMPtr<nsI...> instead of nsRefPtr<ns...> ? It would save QI call with the same effect.
Attachment #372148 - Flags: review?(honzab.moz) → review+
(In reply to comment #29) > Then, why are you using nsCOMPtr<nsI...> instead of nsRefPtr<ns...> ? It would > save QI call with the same effect. There's no QI call in the use in attachment 372148 [details] [diff] [review].
Ah, you are right. Then I have no objections to the patch.
Running with the attached patch appears to fix the problem. This bug is still causing periodic crashes on Windows tinderbox machines, it would be great it we could get this in soon!
Severity: major → critical
Keywords: crash
Attachment #372148 - Flags: superreview?(jst)
Attachment #372148 - Flags: superreview+
Attachment #372148 - Flags: approval1.9.1+
Status: NEW → RESOLVED
Closed: 16 years ago
Keywords: fixed1.9.1
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.2a1
Happened several times yesterday, no crashes after checkin at 23:09 WINNT 5.2 mozilla-central unit test on 2009/04/13 08:04:38 WINNT 5.2 mozilla-central unit test on 2009/04/13 08:26:08 WINNT 5.2 mozilla-central unit test on 2009/04/13 10:23:53 WINNT 5.2 mozilla-central unit test on 2009/04/13 14:38:35 WINNT 5.2 mozilla-central unit test on 2009/04/13 19:58:41 WINNT 5.2 mozilla-central unit test on 2009/04/13 20:22:19
Sounds good to me!
Status: RESOLVED → VERIFIED
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: