edit message as new for messages with attachments corrupts heap

VERIFIED FIXED in Thunderbird 3.1a1


MailNews Core
8 years ago
2 years ago


(Reporter: Bienvenu, Assigned: Bienvenu)


({crash, fixed-seamonkey2.0.3})

Thunderbird 3.1a1
Windows 7
crash, fixed-seamonkey2.0.3

Firefox Tracking Flags

(blocking-thunderbird3.0 .1+, thunderbird3.0 .1-fixed)



(1 attachment)



8 years ago
spun off from bug 312025. This may be debug only, since it seems to be related to the url leak code added to nsStandardUrl.cpp, but I haven't quite pinpointed the actual blame, so I can't be sure it's debug only. And I can't know if this should block bug 312025 until I figure that out.

One difference between the crash and non crash case is that when we edit a message as new, we create a url to save the attachment.

This bug also affects forward inline, which goes through the same mimedrft.cpp code. And in theory, editing a saved draft would have the same issue.

Comment 1

8 years ago
Created attachment 415914 [details] [diff] [review]
proposed fix

The bug is that mime isn't using xpcom reference counting semantics, so it's deleting an object that it should not.  I also cleaned up a separate ref counting issue where mime was addreffing a pointer that had already been addreffed (though only getting rid of the delete fixes the crash).

For some reason, the url leak stuff added to nsStandardUrl.cpp exposed this issue, perhaps by changing the size of the url enough so that we weren't saved by heap buffer size rounding...

I'll try to come up with a mozmill test that at least exercises this code, though it won't guarantee anything...
Attachment #415914 - Flags: superreview?(bugzilla)
Attachment #415914 - Flags: review?(kent)
Attachment #415914 - Flags: approval-thunderbird3.0.1?

Comment 2

8 years ago
Comment on attachment 415914 [details] [diff] [review]
proposed fix

Running with this patch and the patch from bug 312025, I am no longer crashing, while without this patch I crash within three forwards using the test message in that bug.

I also checked the code and its uses, and this seems like the right thing to do.
Attachment #415914 - Flags: review?(kent) → review+
Severity: normal → critical
Keywords: crash

Comment 3

8 years ago
this should block 3.01
blocking-thunderbird3.0: --- → .1+

Comment 4

8 years ago
The changes in this patch are being implemented in trunk as part of bug 312025.
Attachment #415914 - Flags: superreview?(bugzilla) → superreview+

Comment 5

8 years ago
fixed on trunk.
Last Resolved: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 3.1a1
Flags: in-testsuite?
Attachment #415914 - Flags: approval-thunderbird3.0.1? → approval-thunderbird3.0.1+
Whiteboard: [needs landing on branch]

Comment 6

8 years ago
fixed for 3.01
status-thunderbird3.0: --- → .1-fixed
(In reply to comment #6)
> fixed for 3.01

Shouldn't this have landed on 'default' hg branch (too)?

Comment 8

8 years ago
Didn't this land on the trunk a week ago, as I said in #c5? Looks to me like it did.


8 years ago
Whiteboard: [needs landing on branch]
(In reply to comment #8)
> Didn't this land on the trunk a week ago, as I said in #c5? Looks to me like it
> did.

Take a look at: http://hg.mozilla.org/releases/comm-1.9.1/rev/eb1a0eb3b4ef
(and http://hg.mozilla.org/releases/comm-1.9.1/rev/05a86172f79f)

It landed on COMM1915_20091112_RELBRANCH within comm-central rather than "default".

Can you back them out from the relbranch and reland on default please?

Comment 10

8 years ago
(In reply to comment #9)
> Can you back them out from the relbranch and reland on default please?



8 years ago
Duplicate of this bug: 536498


8 years ago
V. Fixed based on the use of the email example pointed by rkent.
Keywords: verified-thunderbird3.0


8 years ago
Keywords: fixed-seamonkey2.0.3
Flags: in-testsuite?
You need to log in before you can comment on or make changes to this bug.