Closed Bug 482432 Opened 15 years ago Closed 14 years ago

crash [@ nsImapFolderCopyState::OnStopRunningUrl(nsIURI*, unsigned int)]

Categories

(MailNews Core :: Networking: IMAP, defect)

1.9.1 Branch
x86
All
defect
Not set
critical

Tracking

(thunderbird3.1 .1-fixed, thunderbird3.0 .6-fixed)

VERIFIED FIXED
Future
Tracking Status
thunderbird3.1 --- .1-fixed
thunderbird3.0 --- .6-fixed

People

(Reporter: Usul, Assigned: Bienvenu)

References

()

Details

(Keywords: crash, regression, topcrash, Whiteboard: [gs][gssolved])

Crash Data

Attachments

(2 files)

0  	thunderbird.exe  	nsImapFolderCopyState::OnStopRunningUrl  	nsImapMailFolder.cpp:6938
1 	thunderbird.exe 	nsMsgMailNewsUrl::SetUrlState 	nsMsgMailNewsUrl.cpp:135
2 	thunderbird.exe 	nsImapMailFolder::SetUrlState 	nsImapMailFolder.cpp:6078
3 	xpcom_core.dll 	NS_InvokeByIndex_P 	xpcom/reflect/xptcall/src/md/win32/xptcinvoke.cpp:101
4 	nspr4.dll 	PR_Unlock 	nsprpub/pr/src/threads/combined/prulock.c:355
5 	xpcom_core.dll 	nsProxyObjectCallInfo::Run 	xpcom/proxy/src/nsProxyEvent.cpp:181
6 	xpcom_core.dll 	nsThread::ProcessNextEvent 	xpcom/threads/nsThread.cpp:510
Flags: wanted-thunderbird3?
"moving folders local to imap" is the comment for bp-dc61121d-2024-4106-afb0-0b0482090305

doesn't appear in TB2/talkback, so assuming this is a regression. earliest crash found is build 20081205075757 bp-3ab59962-462c-4c39-9d3c-d9f062081208 (but it's pretty hard to say that's the first crash because hunting old crashes on crash-stats is dang hard)
Summary: crash @ nsImapFolderCopyState::OnStopRunningUrl → crash [@ nsImapFolderCopyState::OnStopRunningUrl]
Crashes at http://hg.mozilla.org/comm-central/file/6fcaf32e4036/mailnews/imap/src/nsImapMailFolder.cpp#l6938

The one rv is unchecked, though I don't know if it's needed for these iterator methods.
3.0b3 #8 topcrash, so requesting blocking
Flags: wanted-thunderbird3? → blocking-thunderbird3?
Keywords: topcrash
Summary: crash [@ nsImapFolderCopyState::OnStopRunningUrl] → crash [@ nsImapFolderCopyState::OnStopRunningUrl(nsIURI*, unsigned int)]
dropped to #39 in 3.0b3 ranking and doesn't appear in top 100 of 3.0b4pre. so
topcrash- and removing blocking-thunderbird3?
Flags: blocking-thunderbird3?
Keywords: topcrashtopcrash-
yes, that sounds right. If it regains its previous popularity, we can always move it back on.
bienvenu, magnus, do you think we need more information to attempt a fix?

This looks like a pain point for importers of outlook data. see crash comments at https://crash-stats.mozilla.com/query/query?product=Thunderbird&version=ALL%3AALL&date=03%2F16%2F2010&range_value=4&range_unit=weeks&query_search=signature&query_type=exact&query=nsImapFolderCopyState%3A%3AOnStopRunningUrl%28nsIURI*%2C+unsigned+int%29&build_id=&process_type=all&do_query=1

I don't know precisely if it's certain types of folders or data, or whether it's only moving from local to imap account. Outlook imports to local folders, and it's common for user to then move the imported data to a pop or imap account.

The person in #thunderbird yesterday, viaSanctus, experienced this crash. unfortunately no testcase or log from him. Partial IRC log
<wsmwk_away>	i wonder if we are seeing bug 199079
<firebot>	wsmwk_away: Bug https://bugzilla.mozilla.org/show_bug.cgi?id=199079 maj, --, ---, nobody, NEW, Dragging folders either works, does nothing, or copies only the folder (not the mail inside), and of
<viaSanctus>	whilst office 2K7 hangs
<viaSanctus>	another issue i'm having is that the imap store suddenly asks for the password in the middle of the move process
<wsmwk_away>	which unfortunately is not really in good shape/recently confirmed
<bienvenu>	in the middle? has it actually started talking to the imap server?
<bienvenu>	I'm getting the impression the server is dropping the connection on you
<viaSanctus>	perhaps
<viaSanctus>	does office retry the connection?
<viaSanctus>	because I've moved thens of thousands of mails in outlook without problems
<viaSanctus>	tens*
<bienvenu>	no idea...
<bienvenu>	oh, talking to an exchange server?
<bienvenu>	or a real imap server?
<viaSanctus>	mail enable imap
<viaSanctus>	version 4.0 professional
<viaSanctus>	latest release
<viaSanctus>	includes smtp, web and pop
<wsmwk_away>	or rather http://www.mailenable.com/
#19 for v3.0.3, so worthy of "wanted" for 3.1 and changing back to topcrash
blocking-thunderbird3.1: --- → ?
Keywords: topcrash-topcrash
blocking-thunderbird3.1: ? → ---
Flags: wanted-thunderbird+
Attached patch proposed fixSplinter Review
Potentially fixes it.
Assignee: nobody → mkmelin+mozilla
Status: NEW → ASSIGNED
Attachment #433617 - Flags: superreview?(bienvenu)
Attachment #433617 - Flags: review?(bienvenu)
Attachment #433617 - Flags: superreview?(bienvenu)
Attachment #433617 - Flags: superreview-
Attachment #433617 - Flags: review?(bienvenu)
Attachment #433617 - Flags: review-
Comment on attachment 433617 [details] [diff] [review]
proposed fix

I think it would be better not to abort the whole move/copy because we had an error iterating over the db. Instead, I think we should continue to the next message/folder, perhaps with an assert, if possible. Also, we would really like to have a reproducible case for this bug so we could know what exactly is going wrong...
But but, the continuing to next is what's failing (or that's the theory)... How do i go on to next from that?
bienvenu: ^^^
if you copy multiple folders, we copy the messages within each folder, chaining them, and create the target folders. If you bail out like the patch proposes, it breaks the chain, and we won't go to the next folder. It's possible that we did find some messages to copy. Given that we don't know what causes this crash, it's hard to know what the right thing to do is...
Ok, i don't really see what else to do here, so back to defaults.
Assignee: mkmelin+mozilla → nobody
Status: ASSIGNED → NEW
hasMoreElements should be initialized at first.

As long as I disassemble code around crash, I believe that messages seems be NULL.
Attached patch proposed fixSplinter Review
thx, we should definitely be initializing hasMore to false...
Assignee: nobody → bienvenu
Attachment #453376 - Flags: superreview?(neil)
Attachment #453376 - Flags: review?(neil)
Attachment #453376 - Flags: superreview?(neil)
Attachment #453376 - Flags: superreview+
Attachment #453376 - Flags: review?(neil)
Attachment #453376 - Flags: review+
fixed on trunk
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
Target Milestone: --- → Future
Attachment #453376 - Flags: approval-thunderbird3.1.1?
Attachment #453376 - Flags: approval-thunderbird3.1.1?
Attachment #453376 - Flags: approval-thunderbird3.1.1+
Attachment #453376 - Flags: approval-thunderbird3.0.6+
Comment on attachment 453376 [details] [diff] [review]
proposed fix

a=Standard8 please land this for comm-1.9.1 as well as comm-1.9.2 as the crash happens in both places.
fixed for 3.0.x and 3.1.x
Unless we have a good testcase, we wait til 3.1.1 or 3.0.6 ships to verify this is gone because there are no crashes on trunk.

Bienvenu, could checkin http://hg.mozilla.org/comm-central/rev/6521a88c1fc7 have increased the probability of crashing in this area?  Bug 448550 - can't move more than 2 folders to an imap folder.  

And what is the connection to import crashes, which imports to local folders, not imap?  Does import exercise the imap code?

(In reply to comment #17)
> (From update of attachment 453376 [details] [diff] [review])
> a=Standard8 please land this for comm-1.9.1 as well as comm-1.9.2 as the crash
> happens in both places.
>...
> fixed for 3.0.x and 3.1.x

Thanks for that - this is emerging as #1-3 crash early in the 3.1 release. why the increase from not even being top 40 I don't know.  Speculation (based on crash comments) ... more people trying to import.

Lastly, doesn't much matter at this point but crashes go back to at least 20080708 3.0a1 so we're not going to get a regression range. And bug may not even be a regression.
Whiteboard: [verify 3.1.1]
(In reply to comment #19)
> Lastly, doesn't much matter at this point but crashes go back to at least
> 20080708 3.0a1 

correction - 2008050715
(In reply to comment #19)

> 
> Bienvenu, could checkin http://hg.mozilla.org/comm-central/rev/6521a88c1fc7
> have increased the probability of crashing in this area?  Bug 448550 - can't
> move more than 2 folders to an imap folder.  
Yes, I think it very well could have increased the probability. 
> 
> And what is the connection to import crashes, which imports to local folders,
> not imap?  Does import exercise the imap code?

No, it doesn't. But perhaps people try to move folders around after import.
replaced url http://crash-stats.mozilla.com/report/index/dc61121d-2024-4106-afb0-0b0482090305 with gsfn http://gsfn.us/t/189xb

I did not search the rest of gsfn for more reports
Whiteboard: [verify 3.1.1] → [verify 3.1.1][gs]
Does not appear in crash-stats for v3.1.1, v3.1.2 and v3.0.6, so v.fixed
Status: RESOLVED → VERIFIED
Whiteboard: [verify 3.1.1][gs] → [gs][gssolved]
Crash Signature: [@ nsImapFolderCopyState::OnStopRunningUrl(nsIURI*, unsigned int)]
You need to log in before you can comment on or make changes to this bug.