Last Comment Bug 323131 - crash after saving all attachments [@ nsMessenger::SaveAllAttachments]
: crash after saving all attachments [@ nsMessenger::SaveAllAttachments]
: crash, fixed1.8.0.2, fixed1.8.1, verified1.8.1.3
Product: MailNews Core
Classification: Components
Component: Backend (show other bugs)
: Trunk
: x86 All
-- critical with 1 vote (vote)
: ---
Assigned To: David :Bienvenu
: 323427 (view as bug list)
Depends on:
  Show dependency treegraph
Reported: 2006-01-12 02:36 PST by Jakub Kalas
Modified: 2008-07-31 01:21 PDT (History)
7 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---

improve one function in a bad file (4.59 KB, patch)
2006-01-13 02:58 PST, timeless
neil: superreview+
Details | Diff | Splinter Review
the actual fix. (1.07 KB, patch)
2006-01-15 16:33 PST, David :Bienvenu
mscott: superreview+
mscott: approval1.8.0.2+
mscott: approval1.8.1+
Details | Diff | Splinter Review

Description User image Jakub Kalas 2006-01-12 02:36:23 PST
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; cs; rv:1.8) Gecko/20051111 Firefox/1.5
Build Identifier: version 1.5 (20051201)

Delete all attachments from an email and then try to "save all" attachments. Thunderbird crashes.

Reproducible: Always
Comment 1 User image Magnus Melin 2006-01-12 14:00:24 PST
Confirmed on branch and trunk. 

Talkback ids, e.g. TB3882732M, TB3882641X
Comment 2 User image timeless 2006-01-13 02:30:36 PST
you lost a digit from your incident ids, please try not to do that :(
Incident ID: 13882641
Stack Signature	nsMessenger::SaveAllAttachments() 7c0f1f4f
Product ID	Thunderbird2
Build ID	2006010905
Trigger Time	2006-01-12 13:55:45.0
Platform	LinuxIntel
Operating System	Linux 2.6.12-10-386
Module	thunderbird-bin + (0075b157)
URL visited	
User Comments	delete all attachments, then save all. bug 323131
Since Last Crash	5 sec
Total Uptime	5 sec
Trigger Reason	SIGSEGV: Segmentation Fault: (signal 11)
Source File, Line No.	/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/mailnews/base/src/nsMessenger.cpp, line 655
Stack Trace 	
nsMessenger::SaveAllAttachments()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/mailnews/base/src/nsMessenger.cpp, line 655]
nsMessenger::SaveAllAttachments()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/mailnews/base/src/nsMessenger.cpp, line 906]
XPCWrappedNative::CallMethod(XPCCallContext&, XPCWrappedNative::CallMode)()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/js/src/xpconnect/src/xpcwrappednative.cpp, line 3091]
XPC_WN_CallMethod()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/js/src/xpconnect/src/xpcwrappednativejsops.cpp, line 1444]
js_Invoke()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/js/src/jsinterp.c, line 1177]
js_Interpret()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/js/src/jsinterp.c, line 3522]
js_Invoke()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/js/src/jsinterp.c, line 1197]
js_InternalInvoke()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/js/src/jsinterp.c, line 1274]
JS_CallFunctionValue()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/js/src/jsapi.c, line 4158]
nsJSContext::CallEventHandler()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/dom/src/base/nsJSEnvironment.cpp, line 1413]
nsJSEventListener::HandleEvent()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/dom/src/events/nsJSEventListener.cpp, line 185]
nsEventListenerManager::HandleEventSubType()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/content/events/src/nsEventListenerManager.cpp, line 848]
nsEventListenerManager::HandleEvent()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/content/events/src/nsEventListenerManager.cpp, line 1784]
nsXULElement::HandleDOMEvent()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/content/xul/content/src/nsXULElement.cpp, line 2153]
PresShell::HandleDOMEventWithTarget()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/layout/base/nsPresShell.cpp, line 6469]
nsMenuFrame::Execute()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/layout/xul/base/src/nsMenuFrame.cpp, line 848]
nsMenuFrame::HandleEvent()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/layout/xul/base/src/nsMenuFrame.cpp, line 454]
PresShell::HandleEventInternal()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/layout/base/nsPresShell.cpp, line 69]
PresShell::HandleEvent()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/layout/base/nsPresShell.cpp, line 6209]
nsViewManager::HandleEvent()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/view/src/nsViewManager.cpp, line 848]
nsViewManager::DispatchEvent()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/view/src/nsViewManager.cpp, line 2254]
HandleEvent()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/view/src/nsView.cpp, line 251]
nsCommonWidget::DispatchEvent()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/widget/src/gtk2/nsCommonWidget.cpp, line 219]
nsWindow::OnButtonReleaseEvent()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/widget/src/gtk2/nsWindow.cpp, line 1601]
button_release_event_cb()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/widget/src/gtk2/nsWindow.cpp, line 3737] + 0x12002c (0xb7bb402c) + 0x93a8 (0xb79713a8) + 0x17b13 (0xb797fb13) + 0x18ec3 (0xb7980ec3) + 0x194c3 (0xb79814c3) + 0x20216f (0xb7c9616f) + 0x11e767 (0xb7bb2767) + 0x11eba0 (0xb7bb2ba0) + 0x3fb2d (0xb7a56b2d) + 0x244ee (0xb79084ee) + 0x274f6 (0xb790b4f6) + 0x277e3 (0xb790b7e3) + 0x11de65 (0xb7bb1e65)
nsAppShell::Run()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/widget/src/gtk2/nsAppShell.cpp, line 141]
nsAppStartup::Run()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/toolkit/components/startup/src/nsAppStartup.cpp, line 151]
XRE_main()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/toolkit/xre/nsAppRunner.cpp, line 848]
main()  [/builds/tinderbox/Tb-Mozilla1.8/Linux_2.4.18-14_Depend/mozilla/mail/app/nsMailApp.cpp, line 63] + 0x14ea2 (0xb745cea2)

There are all sorts of bugs in the relevant code, but i can't spot the one that actually caused you crash.

this needs to be result checked:

this needs to be oom checked:
saveState = new nsSaveAllAttachmentsState(count,
(const char*) dirName, detaching);

this and following leak saveState:
rv = ConvertAndSanitizeFileName(displayNameArray[0], nsnull, getter_Copies(unescapedName));
if (NS_FAILED(rv))
goto done;

this should be result checked:

most likely these lines crashed:
 970 rv = ConvertAndSanitizeFileName(displayNameArray[0], nsnull, getter_Copies(unescapedName));

 979 rv = SaveAttachment(fileSpec, urlArray[0], messageUriArray[0], 
 980                     contentTypeArray[0], (void *)saveState);

since they aren't checking count and just presume the array has a length
Comment 3 User image timeless 2006-01-13 02:58:12 PST
Created attachment 208364 [details] [diff] [review]
improve one function in a bad file
Comment 4 User image 2006-01-14 13:47:36 PST
Comment on attachment 208364 [details] [diff] [review]
improve one function in a bad file

>     nsresult rv = NS_ERROR_OUT_OF_MEMORY;
>     nsCOMPtr<nsIFilePicker> filePicker =
>         do_CreateInstance(";1", &rv);
Hardly having been set a good example above, but isn't this a useless init?
>+    nsSaveAllAttachmentsState *saveState = nsnull;
>     saveState = new nsSaveAllAttachmentsState(count,
>                                               contentTypeArray,
>                                               urlArray,
>                                               displayNameArray,
>                                               messageUriArray, 
>-                                              (const char*) dirName, detaching);
>+                                              dirName.get(), detaching);
Comment 5 User image allblue 2006-01-15 06:08:22 PST
*** Bug 323427 has been marked as a duplicate of this bug. ***
Comment 6 User image David :Bienvenu 2006-01-15 16:33:44 PST
Created attachment 208609 [details] [diff] [review]
the actual fix.

I believe this should fix the crash.
Comment 7 User image 2006-01-15 17:44:59 PST
It would be great to make the status of detached a lot more obvious, then maybe people wouldn't be trying as much to save/delete a detached file (a very similar situation to this one).  Of course, I am sure that suggestion has been seen in many other places.

Like at:
Comment 8 User image David :Bienvenu 2006-01-20 14:38:59 PST
fixed on trunk, will want for 1.8.1 and perhaps
Comment 9 User image Scott MacGregor 2006-01-26 13:05:37 PST
i think you meant 1.8.1 here adjusting the fixed keyword.
Comment 10 User image David :Bienvenu 2006-01-26 13:07:22 PST
yup, I'd better go look at some other bugs I added that keyword to :-(
Comment 11 User image Scott MacGregor 2006-01-31 12:00:38 PST
Comment on attachment 208609 [details] [diff] [review]
the actual fix.

low risk crash fix, approving.
Comment 12 User image Tracy Walker [:tracy] 2006-03-16 10:24:12 PST
verified fixed Windows Thunderbird 20060308
Comment 13 User image Carsten Book [:Tomcat] 2007-04-05 16:12:45 PDT
verified fixed on using Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv: Gecko/20070326 Thunderbird/ Mnenhy/ ID:2007032620 - no crash on steps to reproduce - adding verify keyword

Note You need to log in before you can comment on or make changes to this bug.