60164 - Out of memory exceptions can cause SEGV when there is no memory recovered from garbage collection.

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect, P3)

Description

[Justin Fletcher]

When an out of memory exception occurs, and there really is no more memory, 
with JS_HAS_ERROR_EXCEPTIONS:

js_ErrorToException attempts to create a new object to hold this exception in. 
This is not checked for failure (and neither is the NewStringCopyZ that 
immediately follows it). This isn't a major problem, however, as the 
js_NewObject creation will never return. The allocation is attempted but 
returns an out of memory error, which causes an exception...
Eventually the code I have runs out of stack and aborts with SEGV.

JavaScript:
function Thing()
{
  this.bingle = 77;
}

lots = new Array;
count = 0;
while (count++ < 20000)
{
  lots[count]=new Thing;
}

JS engine is initialised with runtime=JS_NewRuntime(1024*128), because I wanted 
to check the stability of the engine when memory was tight.

Backtrace whilst the exceptions are occuring show the path as being :
ReportError
JS_ReportErrorNumberVA
JS_ReportErrorNumber
JS_ReportOutOfMemory
js_AllocGCThing
js_NewObject
js_ErrorToException
ReportError

I would expect a problem like this to report an out of memory error without the 
exception mechanism.

Comment 1

[Phil Schwartau]

cc'ing JS Engine team  - compare bug 46196

Comment 2

[Brendan Eich [:brendan]]

What version of Mozilla was this reported against?  I thought rogerl's fix for
46196 went into the trunk. OTOH, the unchecked calls to js_NewObject and
JS_NewStringCopyZ should have been fixed with that bug, but weren't.  So at
least those unchecked calls still need to be fixed.

Phil, did you verify 46196 yet?

/be

Comment 3

[Justin Fletcher]

Apologies; I've just checked against the fixes applied to the trunk and it 
seems my source is outdated with regard to it. Sorry for the duplicate report
:-(

Comment 4

[Brendan Eich [:brendan]]

No worries.  I think this bug should stay open and be used to fix the unchecked
js_NewObject and JS_NewStringCopyZ calls.

/be

Comment 5

[rogerl (gone)]

Adding patch to check newObject and StringCopyZ calls..

Comment 6

[rogerl (gone)]

Attachment: Added missing error checks (and re-wrote another to match)

Comment 7

[Brendan Eich [:brendan]]

mccabe, can you r=?  I'll sr=.

/be

Comment 8

[Mike McCabe]

r=mccabe

Comment 9

[Brendan Eich [:brendan]]

a=[e-mail].

/be

Comment 10

[rogerl (gone)]

Fix checked in.

Comment 11

[Phil Schwartau]

Marking Verified -

Comment 12

[Marek Fort]

*** Bug 70332 has been marked as a duplicate of this bug. ***

Comment 13

[Marek Fort]

*** Bug 70332 has been marked as a duplicate of this bug. ***