Closed Bug 473841 Opened 17 years ago Closed 14 years ago

"ASSERTION: NULL mIconRequest! Multiple calls to OnStopRequest()?"

Categories

(Core :: Widget: Cocoa, defect)

Product:
Core
Component:
Widget: Cocoa
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla6
Tracking Flags:
Tracking Status
blocking2.0 --- -
status2.0 --- ?

People

(Reporter: jruderman, Assigned: joe)

References

Details

(Keywords: assertion, intermittent-failure)

Attachments

(1 file)

remove assert, add condition
791 bytes, patch
jaas
: review+
bzbarsky
: review+
Details | Diff | Splinter Review
Every once in a while, I get: ###!!! ASSERTION: NULL mIconRequest! Multiple calls to OnStopRequest()?: 'mIconRequest', file /Users/jruderman/central/widget/src/cocoa/nsMenuItemIconX.mm, line 481 This is a continuation of bug 471101, which turned a null-dereference crash into an assertion (pending figuring out why it happens). It sounds like figuring out why it happens requires being able to reproduce this bug reliably :/
Assignee: joshmoz → nobody
This just happened randomly on tinderbox: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1260646724.1260651373.18170.gz OS X 10.5.2 mozilla-central leak test build on 2009/12/12 11:38:44 s: moz2-darwin9-slave17 WARNING: NS_ENSURE_TRUE(mMutable) failed: file /builds/slave/mozilla-central-macosx-debug/build/netwerk/base/src/nsSimpleURI.cpp, line 224 ###!!! ASSERTION: NULL mIconRequest! Multiple calls to OnStopRequest()?: 'mIconRequest', file /builds/slave/mozilla-central-macosx-debug/build/widget/src/cocoa/nsMenuItemIconX.mm, line 589 nsMenuItemIconX::OnStopRequest(imgIRequest*, int) (/var/folders/TL/TLg3RrMbFAur2hBCXvCeqk+++TM/-Tmp-//ccP1qjs1.s:256) imgRequestProxy::OnStopRequest(nsIRequest*, nsISupports*, unsigned int, int) (respcmn.c:43) imgRequest::OnStopRequest(nsIRequest*, nsISupports*, unsigned int) (respcmn.c:43) ProxyListener::OnStopRequest(nsIRequest*, nsISupports*, unsigned int) (respcmn.c:43) imgCacheValidator::OnStopRequest(nsIRequest*, nsISupports*, unsigned int) (respcmn.c:43) nsJARChannel::OnStopRequest(nsIRequest*, nsISupports*, unsigned int) (respcmn.c:43) nsInputStreamPump::OnStateStop() (respcmn.c:43) nsInputStreamPump::OnInputStreamReady(nsIAsyncInputStream*) (respcmn.c:43) nsInputStreamReadyEvent::Run() (/var/folders/TL/TLg3RrMbFAur2hBCXvCeqk+++TM/-Tmp-//ccP1qjs1.s:256) nsThread::ProcessNextEvent(int, int*) (/var/folders/TL/TLg3RrMbFAur2hBCXvCeqk+++TM/-Tmp-//ccP1qjs1.s:256) NS_ProcessPendingEvents_P(nsIThread*, unsigned int) (/var/folders/TL/TLg3RrMbFAur2hBCXvCeqk+++TM/-Tmp-//ccP1qjs1.s:256) nsBaseAppShell::NativeEventCallback() (/var/folders/TL/TLg3RrMbFAur2hBCXvCeqk+++TM/-Tmp-//ccP1qjs1.s:256) nsAppShell::ProcessGeckoEvents(void*) (/var/folders/TL/TLg3RrMbFAur2hBCXvCeqk+++TM/-Tmp-//ccP1qjs1.s:256) CFRunLoopRunSpecific (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation) CFRunLoopRunInMode (/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation) RunCurrentEventLoopInMode (/System/Library/Frameworks/Carbon.framework/Versions/A/Frameworks/HIToolbox.framework/Versions/A/HIToolbox) ReceiveNextEventCommon (/System/Library/Frameworks/Carbon.framework/Versions/A/Frameworks/HIToolbox.framework/Versions/A/HIToolbox) BlockUntilNextEventMatchingListInMode (/System/Library/Frameworks/Carbon.framework/Versions/A/Frameworks/HIToolbox.framework/Versions/A/HIToolbox) _DPSNextEvent (/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit) -[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] (/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit) -[NSApplication run] (/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit) nsAppShell::Run() (/var/folders/TL/TLg3RrMbFAur2hBCXvCeqk+++TM/-Tmp-//ccP1qjs1.s:256) nsAppStartup::Run() (respcmn.c:43) XRE_main (respcmn.c:43) main (/builds/slave/mozilla-central-macosx-debug/build/xpcom/glue/nsComponentManagerUtils.cpp:)
Blocks: 438871
Whiteboard: [orange]
http://tinderbox.mozilla.org/showlog.cgi?log=SeaMonkey/1286562276.1286562713.32024.gz&fulltext=1 OS X 10.5 comm-central-trunk debug test crashtest on 2010/10/08 11:24:36 { REFTEST TEST-START | file:///builds/slave/comm-central-trunk-macosx-debug-unittest-crashtest/build/reftest/tests/content/base/crashtests/231475-1.html ###!!! ASSERTION: NULL mIconRequest! Multiple calls to OnStopRequest()?: 'mIconRequest', file /builds/slave/comm-central-trunk-macosx-debug/build/mozilla/widget/src/cocoa/nsMenuItemIconX.mm, line 549 nsMenuItemIconX::OnStopRequest [widget/src/cocoa/nsMenuItemIconX.mm:550] imgRequestProxy::OnStopRequest [modules/libpr0n/src/imgRequestProxy.cpp:710] imgStatusTracker::SendStopRequest [modules/libpr0n/src/imgStatusTracker.cpp:524] imgRequest::OnStopRequest [modules/libpr0n/src/imgRequest.cpp:959] ProxyListener::OnStopRequest [modules/libpr0n/src/imgLoader.cpp:2008] imgCacheValidator::OnStopRequest [modules/libpr0n/src/imgLoader.cpp:2164] nsJARChannel::OnStopRequest [modules/libjar/nsJARChannel.cpp:907] nsInputStreamPump::OnStateStop [netwerk/base/src/nsInputStreamPump.cpp:579] nsInputStreamPump::OnInputStreamReady [netwerk/base/src/nsInputStreamPump.cpp:403] nsInputStreamReadyEvent::Run [xpcom/io/nsStreamUtils.cpp:113] nsThread::ProcessNextEvent [xpcom/threads/nsThread.cpp:547] NS_ProcessPendingEvents_P [nsThreadUtils.cpp:200] nsBaseAppShell::NativeEventCallback [widget/src/xpwidgets/nsBaseAppShell.cpp:132] nsAppShell::ProcessGeckoEvents [widget/src/cocoa/nsAppShell.mm:395] CoreFoundation + 0x733c5 CoreFoundation + 0x73aa8 HIToolbox + 0x302ac HIToolbox + 0x300c5 HIToolbox + 0x2ff39 AppKit + 0x406d5 AppKit + 0x3ff88 AppKit + 0x38f9f nsAppShell::Run [widget/src/cocoa/nsAppShell.mm:747] nsAppStartup::Run [toolkit/components/startup/src/nsAppStartup.cpp:191] XRE_main [toolkit/xre/nsAppRunner.cpp:3670] main [suite/app/nsSuiteApp.cpp:103] seamonkey-bin + 0x170e REFTEST TEST-PASS | file:///builds/slave/comm-central-trunk-macosx-debug-unittest-crashtest/build/reftest/tests/content/base/crashtests/231475-1.html | (LOAD ONLY) ... REFTEST TEST-UNEXPECTED-FAIL | file:///builds/slave/comm-central-trunk-macosx-debug-unittest-crashtest/build/reftest/tests/content/base/crashtests/231475-1.html | assertion count 1 is more than expected 0 assertions }
"blocking2.0=?": Though intermittent, this seems to happen frequently enough that it should be reproducible to analyze.
blocking2.0: --- → ?
I agree we should investigate it, but doing so doesn't block 2.0.
blocking2.0: ? → -
Joe, I notice that nsMenuItemIconX::LoadIcon and nsMenuItemIconX::OnStopRequest both call mIconRequest->Cancel instead of the synchronous CancelAndForgetObserver in use elsewhere in the file. Since Cancel is asynchronous, presumably we would null out mIconRequest after calling it, and we could end up triggering the assertion when the final OnStopRequest comes through. I'm not sure what the stacks would look like in this case - does the one in comment 1 match up with my idea?
(In reply to comment #13) > Joe, I notice that nsMenuItemIconX::LoadIcon and nsMenuItemIconX::OnStopRequest > both call mIconRequest->Cancel instead of the synchronous > CancelAndForgetObserver in use elsewhere in the file. Since Cancel is > asynchronous, presumably we would null out mIconRequest after calling it, and > we could end up triggering the assertion when the final OnStopRequest comes > through. I'm not sure what the stacks would look like in this case - does the > one in comment 1 match up with my idea? Joe, any ideas?
(In reply to comment #13) > Joe, I notice that nsMenuItemIconX::LoadIcon and nsMenuItemIconX::OnStopRequest > both call mIconRequest->Cancel instead of the synchronous > CancelAndForgetObserver in use elsewhere in the file. Since Cancel is > asynchronous, presumably we would null out mIconRequest after calling it, and > we could end up triggering the assertion when the final OnStopRequest comes > through. To be clear, are you asking whether OnStopRequest could come in after LoadIcon? Yes, that sounds likely. We can't just blindly switch to CancelAndForgetObserver, though, since that's synchronous and thus re-enters ourselves. In this case it might be OK, though - Boris, do you have any ideas? The only way that I can see this happening is if we have a current request that hasn't started loading, then try to load a separate URI that doesn't load properly (because we know mIconRequest is null).
> Boris, do you have any ideas? How about we just fix the broken assertion?
Presuming the description in comment 13 and comment 17 is correct, this seems like the right solution. It will ensure that we only care about OnStopRequest calls from our current icon request.
Assignee: nobody → joe
Attachment #529211 - Flags: review?(joshmoz)
Attachment #529211 - Flags: review?(bzbarsky)
Attachment #529211 - Flags: review?(joshmoz) → review+
Comment on attachment 529211 [details] [diff] [review] remove assert, add condition r=me
Attachment #529211 - Flags: review?(bzbarsky) → review+
http://hg.mozilla.org/mozilla-central/rev/66f16e7dca6b
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla6
Whiteboard: [orange]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: