582165 - crash [@ nsImapProtocol::SetupWithUrl ] with null m_transport [Mac]

Status: RESOLVED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[Ludovic Hirlimann [:Usul]]

Crashed after waking up from sleep :

0  	thunderbird-bin  	nsImapProtocol::SetupWithUrl  	 mailnews/imap/src/nsImapProtocol.cpp:886
1 	thunderbird-bin 	nsImapProtocol::LoadImapUrl 	mailnews/imap/src/nsImapProtocol.cpp:2014
2 	thunderbird-bin 	nsImapIncomingServer::RetryUrl 	mailnews/imap/src/nsImapIncomingServer.cpp:490
3 	libxpcom_core.dylib 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/unix/xptcinvoke_unixish_x86.cpp:179
4 	libxpcom_core.dylib 	nsProxyObjectCallInfo::Run 	xpcom/proxy/src/nsProxyEvent.cpp:181
5 	libxpcom_core.dylib 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:547
6 	libxpcom_core.dylib 	NS_ProcessPendingEvents_P 	nsThreadUtils.cpp:200
7 	thunderbird-bin 	nsBaseAppShell::NativeEventCallback 	widget/src/xpwidgets/nsBaseAppShell.cpp:126
8 	thunderbird-bin 	nsAppShell::ProcessGeckoEvents 	widget/src/cocoa/nsAppShell.mm:394
9 	CoreFoundation 	CFRunLoopRunSpecific 	
10 	CoreFoundation 	CFRunLoopRunInMode 	
11 	HIToolbox 	RunCurrentEventLoopInMode 	
12 	HIToolbox 	ReceiveNextEventCommon 	
13 	HIToolbox 	BlockUntilNextEventMatchingListInMode 	
14 	AppKit 	_DPSNextEvent 	
15 	AppKit 	-[NSApplication nextEventMatchingMask:untilDate:inMode:dequeue:] 	
16 	AppKit 	-[NSApplication run] 	
17 	thunderbird-bin 	nsAppShell::Run 	widget/src/cocoa/nsAppShell.mm:747
18 	thunderbird-bin 	nsAppStartup::Run 	toolkit/components/startup/src/nsAppStartup.cpp:191
19 	thunderbird-bin 	XRE_main 	toolkit/xre/nsAppRunner.cpp:3625
20 	thunderbird-bin 	main 	mail/app/nsMailApp.cpp:101
21 	thunderbird-bin 	thunderbird-bin@0x1b75 	
22 		@0x1

Comment 1

[Makoto Kato [:m_kato]]

m_transport is null.  Should we return error when we cannt get m_transport?

Comment 3

[Makoto Kato [:m_kato]]

this should not be dup.

bug 558452
==========
The crash (bp-49c34c1a-a481-46e7-a00d-7362b2100105) is m_runningUrl is null becasue aURL doesn't have nsIImapUrl interface.

bug 582165 (this)
=================
m_transport is gone away by suspend?, so m_transport is null.

Comment 4

[Makoto Kato [:m_kato]]

This is regression by bug 529921

Comment 5

[Philip Prindeville]

Recommend:

885  	        if (NS_SUCCEEDED(rv) && m_transport)
886 	          m_transport->SetQoSBits(qos);


as a fix.

Comment 6

[David :Bienvenu]

Attachment: move transport qos code inside non-null transport branch

this would seem like the low risk fix.

Comment 7

[David :Bienvenu]

might want this for 3.1.2

Comment 8

[Philip Prindeville]

(In reply to comment #6)
> Created attachment 460980 [details] [diff] [review]
> move transport qos code inside non-null transport branch
> this would seem like the low risk fix.

Could of questions.

(1) under what circumstances is m_transport not going to be set, and
(2) why does m_mockChannel also need to be set?

Comment 9

[David :Bienvenu]

(In reply to comment #8)
> (In reply to comment #6)
> > Created attachment 460980 [details] [diff] [review] [details]
> > move transport qos code inside non-null transport branch
> > this would seem like the low risk fix.
> 
> Could of questions.
> 
> (1) under what circumstances is m_transport not going to be set, and

If there's no url, or we have an error creating the transport. In either case, if there's no transport, there's no place to set the bits, right?

> (2) why does m_mockChannel also need to be set?

We're not going to run the url if there's no mock channel. nsImapProtocol requires there to be a channel. Necko requires there to be a channel. So we're not going to have a case where there's a transport run w/o a mock channel, and thus, it doesn't matter what the qos bits are on the transport.

Comment 10

[Mark Banner (:standard8)]

Comment on attachment 460980 [details] [diff] [review]
move transport qos code inside non-null transport branch

r/sr + a=Standard8. Please land on COMM1927_20100730_RELBRANCH as well as default on comm-1.9.2.

Comment 11

[David :Bienvenu]

Comment on attachment 460980 [details] [diff] [review]
move transport qos code inside non-null transport branch

my mistake; this is trunk only.

Comment 12

[David :Bienvenu]

fixed on trunk.