323131 - crash after saving all attachments [@ nsMessenger::SaveAllAttachments]

Status: RESOLVED FIXED

(MailNews Core :: Backend, defect)

Description

[Jakub Kalas]

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

[Magnus Melin [:mkmelin]]

Confirmed on branch and trunk. 

Talkback ids, e.g. TB3882732M, TB3882641X

Comment 2

[timeless]

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]
XPTC_InvokeByIndex()
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]
libgtk-x11-2.0.so.0 + 0x12002c (0xb7bb402c)
libgobject-2.0.so.0 + 0x93a8 (0xb79713a8)
libgobject-2.0.so.0 + 0x17b13 (0xb797fb13)
libgobject-2.0.so.0 + 0x18ec3 (0xb7980ec3)
libgobject-2.0.so.0 + 0x194c3 (0xb79814c3)
libgtk-x11-2.0.so.0 + 0x20216f (0xb7c9616f)
libgtk-x11-2.0.so.0 + 0x11e767 (0xb7bb2767)
libgtk-x11-2.0.so.0 + 0x11eba0 (0xb7bb2ba0)
libgdk-x11-2.0.so.0 + 0x3fb2d (0xb7a56b2d)
libglib-2.0.so.0 + 0x244ee (0xb79084ee)
libglib-2.0.so.0 + 0x274f6 (0xb790b4f6)
libglib-2.0.so.0 + 0x277e3 (0xb790b7e3)
libgtk-x11-2.0.so.0 + 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]
libc.so.6 + 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:
filePicker->Init(mWindow,
GetString(NS_LITERAL_STRING("SaveAllAttachments")),
nsIFilePicker::modeGetFolder);

this needs to be oom checked:
saveState = new nsSaveAllAttachmentsState(count,
contentTypeArray,
urlArray,
displayNameArray,
messageUriArray, 
(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:
fileSpec->SetFromFileSpec(aFileSpec);

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

[timeless]

Attachment: improve one function in a bad file

Comment 4

[[e-mail]]

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("@mozilla.org/filepicker;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

[allblue]

*** Bug 323427 has been marked as a duplicate of this bug. ***

Comment 6

[David :Bienvenu]

Attachment: the actual fix.

I believe this should fix the crash.

Comment 7

[mozilla.org]

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:
https://bugzilla.mozilla.org/show_bug.cgi?id=292385

Comment 8

[David :Bienvenu]

fixed on trunk, will want for 1.8.1 and perhaps 1.8.0.2

Comment 9

[Scott MacGregor]

i think you meant 1.8.1 here adjusting the fixed keyword.

Comment 10

[David :Bienvenu]

yup, I'd better go look at some other bugs I added that keyword to :-(

Comment 11

[Scott MacGregor]

Comment on attachment 208609 [details] [diff] [review]
the actual fix.

low risk crash fix, approving.

Comment 12

[Tracy Walker [:tracy]]

verified fixed Windows Thunderbird 20060308

Comment 13

[Carsten Book [:Tomcat]]

verified fixed on 1.8.1.3 using Mozilla/5.0 (Windows; U; Windows NT 5.2; en-US; rv:1.8.1.3) Gecko/20070326 Thunderbird/2.0.0.0 Mnenhy/0.7.5.0 ID:2007032620 - no crash on steps to reproduce - adding verify keyword