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

Status: RESOLVED WONTFIX

(Tamarin Graveyard :: Virtual Machine, defect)

Description

[Steven Johnson]

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<>).

Comment 1

[Steven Johnson]

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....

Comment 2

[Steven Johnson]

Attachment: 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.

Comment 3

[Lars T Hansen]

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

[Lars T Hansen]

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.)

Comment 5

[Edwin Smith]

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.