Silent ignoring of out-of-memory in jslock.c

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Igor Bukanov, Assigned: Igor Bukanov)

Tracking

Trunk
Points:
---
Bug Flags:
in-testsuite -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Assignee)

Description

11 years ago
Currently, when js_UndependString fails due to faile malloc, jslock.c does not report its failure and instead tries to implement a workaround to recover from this. It would be nice if the failure would be at least reported if not propagated.
(Assignee)

Updated

11 years ago
Blocks: 363059
(Assignee)

Comment 1

11 years ago
Created attachment 248744 [details] [diff] [review]
Fix v1

The patch changes the code to pass JSContext *cx to MakeStringImmutable in jslock.c on all code paths so js_UndependString receives cx, not NULL.
Attachment #248744 - Flags: review?(brendan)
(Assignee)

Comment 2

11 years ago
Created attachment 248746 [details] [diff] [review]
Fix v2

This version removes FIXME comments for this bug and wraps js_Dequeue inside NSPR_LOCK macros so GCC would not complain about a declared but never used function.
Attachment #248744 - Attachment is obsolete: true
Attachment #248746 - Flags: review?(brendan)
Attachment #248744 - Flags: review?(brendan)
Comment on attachment 248746 [details] [diff] [review]
Fix v2

Looks good, thanks -- worth a comment that only only OOM is usefully reported, since other errors turn into exceptions which are lost due to lack of propagated false return from js_FinishSharingScope?

/be
Attachment #248746 - Flags: review?(brendan) → review+
(Assignee)

Comment 4

11 years ago
Created attachment 248756 [details] [diff] [review]
Fix v2b

Better comments in js_FinishSharingScope.
Attachment #248746 - Attachment is obsolete: true
Attachment #248756 - Flags: review?(brendan)
Comment on attachment 248756 [details] [diff] [review]
Fix v2b

>+             * FIXME bug 363059: The following error recovery changes the
>+             * execution semantic arbitrary and silently ignores any errors
>+             * except out-of-memory which should be already reported through
>+             * JS_ReportOutOfMemory at this point.

Comma before "which", and "have been reported" rather than "be already reported", and r=me.

/be
Attachment #248756 - Flags: review?(brendan) → review+
(Assignee)

Comment 6

11 years ago
Created attachment 248797 [details] [diff] [review]
Fix v2c

Patch to commit with English usage fixes.
Attachment #248756 - Attachment is obsolete: true
Attachment #248797 - Flags: review+
(Assignee)

Comment 7

11 years ago
I committed the patch from comment 6 to the trunk:

Checking in jsapi.c;
/cvsroot/mozilla/js/src/jsapi.c,v  <--  jsapi.c
new revision: 3.295; previous revision: 3.294
done
Checking in jslock.c;
/cvsroot/mozilla/js/src/jslock.c,v  <--  jslock.c
new revision: 3.60; previous revision: 3.59
done
Checking in jslock.h;
/cvsroot/mozilla/js/src/jslock.h,v  <--  jslock.h
new revision: 3.33; previous revision: 3.32
done
Checking in jsstr.c;
/cvsroot/mozilla/js/src/jsstr.c,v  <--  jsstr.c
new revision: 3.137; previous revision: 3.136
done
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Updated

11 years ago
Flags: in-testsuite-
You need to log in before you can comment on or make changes to this bug.