Closed Bug 792957 Opened 12 years ago Closed 12 years ago

"ASSERTION: Attempting to allocate excessively large array" with MozLockOrientation

Categories

(Core :: DOM: Core & HTML, defect)

defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: jruderman, Assigned: mounir)

References

Details

(Keywords: assertion, testcase, Whiteboard: [fixed by bug 793244])

Attachments

(3 files)

Attached file testcase
###!!! ASSERTION: Attempting to allocate excessively large array: 'Error', file nsTArray-inl.h, line 112

Maybe this assertion is just bogus.
Attached file stack+
> Maybe this assertion is just bogus.
>
> #4  0x00000001011f72b6 in nsTArray_base<nsTArrayDefaultAllocator>::EnsureCapacity 
> (this=0x7fff5fbf8f40, capacity=183420626, elemSize=16) at nsTArray-inl.h:112

That looks excessively large to me!
This code needs to use a fallible array, at the very least, no?

Or better yet, we should convert nsScreen to WebIDL, which does this sort of thing right... ;)
(In reply to Justin Lebar [:jlebar] from comment #2)
> > Maybe this assertion is just bogus.
> >
> > #4  0x00000001011f72b6 in nsTArray_base<nsTArrayDefaultAllocator>::EnsureCapacity 
> > (this=0x7fff5fbf8f40, capacity=183420626, elemSize=16) at nsTArray-inl.h:112
> 
> That looks excessively large to me!

This is because the js code is passing an array of that size.

I think we should probably not size the array too early. We might actually want a set and in that case, the set will never be higher than 4 entries, whatever the input is.
On the other hand, I'm not sure it make sense to assert here.  We should just fail the capacity set and move on.  Moving on in the infallible array case should mean aborting, by the way!  That seems like a bug in the array code.
OS: Mac OS X → All
Hardware: x86_64 → All
Attached patch PatchSplinter Review
Use a std::set<nsString> instead of a nsTArray. That way, we will never allocate more elements than needed.
I haven't use nsTHashtable because that API is annoying (you have to iterate trough another function) and I see no reason why we shouldn't be allowed to use std::set in our codebase.
Assignee: nobody → mounir
Status: NEW → ASSIGNED
Attachment #663347 - Flags: review?(justin.lebar+bug)
Does std::set ever throw exceptions?
Comment on attachment 663347 [details] [diff] [review]
Patch

Ehsan, do you know if it's safe to use std::set in our code?  If not, can you put this f? to someone who you think might know?
Attachment #663347 - Flags: feedback?(ehsan)
Comment on attachment 663347 [details] [diff] [review]
Patch

Review of attachment 663347 [details] [diff] [review]:
-----------------------------------------------------------------

std::set is fine.
Attachment #663347 - Flags: feedback?(ehsan) → feedback+
How does this fix anything? We're just hitting OOM in std::set and hope nothing blows up?
Comment on attachment 663347 [details] [diff] [review]
Patch

I'm with Ms2ger on this one; why not build the bitmap without ever building the set/list?
Attachment #663347 - Flags: review?(justin.lebar+bug) → review-
Bug 793244 is going to fix this.
Depends on: 793244
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Whiteboard: [fixed by bug 793244]
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: