Vector used to throw OutOfMemory error, now it just OOM's

RESOLVED WONTFIX

Status

Tamarin
Virtual Machine
RESOLVED WONTFIX
7 years ago
7 years ago

People

(Reporter: Steven Johnson, Assigned: Steven Johnson)

Tracking

Details

(Whiteboard: WE:#2785364)

Attachments

(1 attachment)

(Assignee)

Description

7 years ago
Given code like:

   var tLines:Vector.<TextLine> = new Vector.<TextLine>(); 
   var value:int = -1; 
   tLines.length = value;

Flash 10.0 would (generall) throw kOutOfMemoryError; at the heart of its memory allocation, it would do something like

	Atom* newArray = (Atom*) gc->Calloc(newCapacity, sizeof(Atom), GC::kContainsPointers);
	if (!newArray)
		toplevel()->throwError(kOutOfMemoryError);

The rewrite of Vector<> in FP10.1 and later, uses List<>, which never even attempts to throw such an error; it simply OOMs. (Indeed, it doesn't have ready access to a Toplevel, so it can't throw even if it wants to.)

This is a Watson bug that they want fixed for Serrano; we need to evaluate whether we want to fix this, and if so, how we're going to (it's going to be tricky to insert the proper plumbing into the guts of List<>).
(Assignee)

Comment 1

7 years ago
Note that we could trivially fix this *specific* bug by preflighting the list-grow cases, and causing any obvious too-large requests to throw rather than OOM, but there will almost certainly be other cases that would have thrown previously that now simply OOM. Still, this might be acceptable behavior....
(Assignee)

Comment 2

7 years ago
Created attachment 520798 [details] [diff] [review]
Obnoxious fix...

Obnoxious-compliance fix that fixes this specific bug. There are plenty of other ways to trigger the "wrong" behavior though: any way that grows a Vector to huge size without an explicit call to ".length=HUGE" will still OOM.
Assignee: nobody → stejohns
Attachment #520798 - Flags: feedback?(lhansen)

Comment 3

7 years ago
I'm not sure this should be fixed.  We had a big meeting about this last year and the realization was that memory errors were thrown unreliably and frequently did not work at all (crashes); ergo it was silly to continue to support it (little or no working code could possibly be depending on it).  Lee was in the room for that decision and was definitely awake.

Comment 4

7 years ago
Comment on attachment 520798 [details] [diff] [review]
Obnoxious fix...

This is a fine fix if you want to support this particular use case.  (It strikes me as an invalid use case though.)
Attachment #520798 - Flags: feedback?(lhansen) → feedback+

Comment 5

7 years ago
based on the earlier decision this sounds like not-a-bug, but instead a broken test case.  We might have to have a recap of the same meeting with the same stakeholders, just to cement the OOM strategy.  kOutOfMemoryError needs a rational policy that we can explain in a spec, or should just be phased out.
(Assignee)

Updated

7 years ago
Status: NEW → RESOLVED
Last Resolved: 7 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.