466318 - crash [@ nsProxyObjectCallInfo::RefCountInInterfacePointers(int)] and variation [@ @0x14c0] ... [@ nsProxyObjectCallInfo::RefCountInInterfacePointers]

Status: VERIFIED FIXED

(MailNews Core :: Networking: IMAP, defect)

Description

[Wayne Mery (:wsmwk)]

crash [@ nsProxyObjectCallInfo::RefCountInInterfacePointers] and variation [@ @0x14c0] ... [@ nsProxyObjectCallInfo::RefCountInInterfacePointers]

(not sure the variation is the same issue)
from crash-stats, current trunk

bp-a492be5f-1fc2-404a-bd38-d38112081121
nsProxyObjectCallInfo::RefCountInInterfacePointers	 xpcom/proxy/src/nsProxyEvent.cpp:208
nsProxyObjectCallInfo::nsProxyObjectCallInfo	xpcom/proxy/src/nsProxyEvent.cpp:145
nsProxyEventObject::CallMethod	xpcom/proxy/src/nsProxyEventObject.cpp:233
PrepareAndDispatch	xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp:114
SharedStub	xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp:141
nsImapProtocol::RetryUrl	nsImapProtocol.cpp:1743 


bp-35040a8e-e773-4205-8c7c-7a8cb2081120
@0x14c0	
nsProxyObjectCallInfo::nsProxyObjectCallInfo	xpcom/proxy/src/nsProxyEvent.cpp:145
nsProxyEventObject::CallMethod	xpcom/proxy/src/nsProxyEventObject.cpp:233
PrepareAndDispatch	xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp:114
SharedStub	xpcom/reflect/xptcall/src/md/win32/xptcstubs.cpp:141
nsImapProtocol::RetryUrl	nsImapProtocol.cpp:1680

Comment 1

[David :Bienvenu]

I believe this is caused by the channel object getting deleted out from under us, despite us holding a strong reference to it. It's very hard to reproduce, which makes it hard to debug, and fix.

Comment 2

[Wayne Mery (:wsmwk)]

no longer topcrash, ranked 60+

related?
bp-178f72b3-3ce3-4f32-a728-229ec2081224
nsMsgComposeAndSend::GetBodyFromEditor	nsMsgSend.cpp:1715
nsProxyObjectCallInfo::RefCountInInterfacePointers	xpcom/proxy/src/nsProxyEvent.cpp:208
xpcom/proxy/src/nsProxyEvent.cpp:145
xpcom/proxy/src/nsProxyEventObject.cpp:233
PrepareAndDispatch
SharedStub
nsImapProtocol::RetryUrl	nsImapProtocol.cpp:1746

Comment 3

[David :Bienvenu]

I doubt that's related, but since I don't have a perfect understanding, I can't say for sure. Marking blocking plus for now to keep on radar.

Comment 4

[Wayne Mery (:wsmwk)]

ludo - this is a topcrash on b1 based on last 4 days.

Comment 5

[David :Bienvenu]

Attachment: possible fix

this is a bit of a stab in the dark, but it's worth a try. It forces the do_QueryReferent to happen on the ui thread, in case we were running into thread-safety issues where the channel was destroyed on the ui thread while we were query referenting it on the imap thread. I'll see if I run into this crash with the patch installed. So far, so good, though this crash is relatively rare for me, except when it's not :-) I've added some assertions to make sure things that should only happen on the ui thread are only happening there.

Comment 6

[Mark Banner (:standard8)]

Comment on attachment 367318 [details] [diff] [review]
possible fix

>+  /*
>+   * Prepare to retry the current url
>+   *
>+   * @param  aImapUrl - url we're going to retry
>+   *
>+   * @returns - channel to associate with the url. We return this
>+   *            because access to the channel should only happen on
>+   *            the ui thread.
>+   */
>+  nsIImapMockChannel prepareToRetryUrl(in nsIImapUrl aImapUrl);
>   /*

nit: blank line after function definition please.


>+    nsresult rv = m_imapServerSink->PrepareToRetryUrl(kungFuGripImapUrl, getter_AddRefs(saveMockChannel));
>+  // the channel might have gone away - we'll just not retry in that case.
>+    NS_ENSURE_SUCCESS(rv, PR_FALSE);
>+  }

nit: incorrect indentation.

Comment 7

[David :Bienvenu]

fix checked in.

Comment 8

[Wayne Mery (:wsmwk)]

this is gone from crash-stats as of 20090318 build (which were all Mac crashes for the 7 days prior to being fixed)
v.fixed

Comment 9

[Wayne Mery (:wsmwk)]

fix signature for crash-stats