Closed
Bug 802459
Opened 12 years ago
Closed 12 years ago
OOM crash if attaching too large file for filelink
Categories
(Thunderbird :: FileLink, defect)
Tracking
(thunderbird17 fixed, thunderbird18 fixed)
RESOLVED
FIXED
Thunderbird 19.0
People
(Reporter: m_kato, Assigned: mconley)
Details
(Keywords: crash)
Crash Data
Attachments
(1 file)
3.22 KB,
patch
|
m_kato
:
review+
standard8
:
approval-comm-aurora+
standard8
:
approval-comm-beta+
|
Details | Diff | Splinter Review |
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);
Assignee | ||
Comment 1•12 years ago
|
||
(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•12 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.
Assignee | ||
Comment 3•12 years ago
|
||
Does this look alright?
Assignee: nobody → mconley
Attachment #675232 -
Flags: review?(m_kato)
Reporter | ||
Updated•12 years ago
|
Attachment #675232 -
Flags: review?(m_kato) → review+
Assignee | ||
Updated•12 years ago
|
status-thunderbird17:
--- → affected
status-thunderbird18:
--- → affected
Assignee | ||
Comment 4•12 years ago
|
||
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 5•12 years ago
|
||
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+
Comment 6•12 years ago
|
||
This was checked in last week: https://hg.mozilla.org/comm-central/rev/70e854549186
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 19.0
Comment 7•12 years ago
|
||
https://hg.mozilla.org/releases/comm-aurora/rev/a71014233c1f https://hg.mozilla.org/releases/comm-beta/rev/d0a73868cef8
Comment 8•12 years ago
|
||
what might one expect to see in crash signature for this OOM crash?
Reporter | ||
Updated•12 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.
Description
•