Note: There are a few cases of duplicates in user autocompletion which are being worked on.

OOM crash if attaching too large file for filelink

RESOLVED FIXED in Thunderbird 19.0

Status

Thunderbird
FileLink
--
critical
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: m_kato, Assigned: mconley)

Tracking

({crash})

16 Branch
Thunderbird 19.0
x86
Windows 7
crash

Thunderbird Tracking Flags

(thunderbird17 fixed, thunderbird18 fixed)

Details

(crash signature)

Attachments

(1 attachment)

(Reporter)

Description

5 years ago
I have fixed Gecko side by bug 801483 if nsIBufferedInputStream.init(<too large size) causes OOM crashes (ex. bp-f7294fd5-e5d4-4c5f-81ea-86b722121012).  So init(<too large size>) in Gecko 19 will throw NS_ERROR_OUT_OF_MEMORY instead of crash.

But, buffer length parameter of init should not use file size because this is buffer size for seeking.

the following code is incorrect usages of nsIBufferedInputStream.

/mail/components/cloudfile/nsUbuntuOne.js (View Hg log or Hg annotations)
    line 692 -- createInstance(Ci.nsIBufferedInputStream);
/mail/components/cloudfile/nsYouSendIt.js (View Hg log or Hg annotations)
    line 1074 -- .createInstance(Ci.nsIBufferedInputStream);
/mail/components/cloudfile/nsBox.js (View Hg log or Hg annotations)
    line 857 -- .createInstance(Ci.nsIBufferedInputStream);
Keywords: crash
(In reply to Makoto Kato from comment #0)
> I have fixed Gecko side by bug 801483 if nsIBufferedInputStream.init(<too
> large size) causes OOM crashes (ex.
> bp-f7294fd5-e5d4-4c5f-81ea-86b722121012).  So init(<too large size>) in
> Gecko 19 will throw NS_ERROR_OUT_OF_MEMORY instead of crash.
> 
> But, buffer length parameter of init should not use file size because this
> is buffer size for seeking.
> 
> the following code is incorrect usages of nsIBufferedInputStream.
> 
> /mail/components/cloudfile/nsUbuntuOne.js (View Hg log or Hg annotations)
>     line 692 -- createInstance(Ci.nsIBufferedInputStream);
> /mail/components/cloudfile/nsYouSendIt.js (View Hg log or Hg annotations)
>     line 1074 -- .createInstance(Ci.nsIBufferedInputStream);
> /mail/components/cloudfile/nsBox.js (View Hg log or Hg annotations)
>     line 857 -- .createInstance(Ci.nsIBufferedInputStream);

Hey Makoto,

Thanks for letting us know about this.

So what is an appropriate buffer size? I'm seeing 4096 being tossed around here: http://mxr.mozilla.org/comm-central/source/mozilla/netwerk/test/unit/test_file_protocol.js#33

Would that be sufficient?

-Mike
(Reporter)

Comment 2

5 years ago
(In reply to Mike Conley (:mconley) from comment #1)
> (In reply to Makoto Kato from comment #0)
> > I have fixed Gecko side by bug 801483 if nsIBufferedInputStream.init(<too
> > large size) causes OOM crashes (ex.
> > bp-f7294fd5-e5d4-4c5f-81ea-86b722121012).  So init(<too large size>) in
> > Gecko 19 will throw NS_ERROR_OUT_OF_MEMORY instead of crash.
> > 
> > But, buffer length parameter of init should not use file size because this
> > is buffer size for seeking.
> > 
> > the following code is incorrect usages of nsIBufferedInputStream.
> > 
> > /mail/components/cloudfile/nsUbuntuOne.js (View Hg log or Hg annotations)
> >     line 692 -- createInstance(Ci.nsIBufferedInputStream);
> > /mail/components/cloudfile/nsYouSendIt.js (View Hg log or Hg annotations)
> >     line 1074 -- .createInstance(Ci.nsIBufferedInputStream);
> > /mail/components/cloudfile/nsBox.js (View Hg log or Hg annotations)
> >     line 857 -- .createInstance(Ci.nsIBufferedInputStream);
> 
> Hey Makoto,
> 
> Thanks for letting us know about this.
> 
> So what is an appropriate buffer size? I'm seeing 4096 being tossed around
> here:
> http://mxr.mozilla.org/comm-central/source/mozilla/netwerk/test/unit/
> test_file_protocol.js#33
> 
> Would that be sufficient?

Yes, sufficient. nsXMLHttpRequest uses same size, too.
Created attachment 675232 [details] [diff] [review]
Patch v1

Does this look alright?
Assignee: nobody → mconley
Attachment #675232 - Flags: review?(m_kato)
(Reporter)

Updated

5 years ago
Attachment #675232 - Flags: review?(m_kato) → review+
status-thunderbird17: --- → affected
status-thunderbird18: --- → affected
Comment on attachment 675232 [details] [diff] [review]
Patch v1

This is a cheap win - do we want it for Aurora and Beta?
Attachment #675232 - Flags: approval-comm-beta?
Attachment #675232 - Flags: approval-comm-aurora?
Comment on attachment 675232 [details] [diff] [review]
Patch v1

Definitely. a=me.
Attachment #675232 - Flags: approval-comm-beta?
Attachment #675232 - Flags: approval-comm-beta+
Attachment #675232 - Flags: approval-comm-aurora?
Attachment #675232 - Flags: approval-comm-aurora+
This was checked in last week:

https://hg.mozilla.org/comm-central/rev/70e854549186
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 19.0
https://hg.mozilla.org/releases/comm-aurora/rev/a71014233c1f
https://hg.mozilla.org/releases/comm-beta/rev/d0a73868cef8
status-thunderbird17: affected → fixed
status-thunderbird18: affected → fixed
what might one expect to see in crash signature for this OOM crash?
(Reporter)

Updated

5 years ago
Crash Signature: [@ mozalloc_abort(char const* const) | mozalloc_handle_oom(unsigned int) | moz_xmalloc | nsBufferedStream::Init(nsISupports*, unsigned int)]
You need to log in before you can comment on or make changes to this bug.