Closed Bug 357392 Opened 18 years ago Closed 18 years ago

jsdtoa.c - locks not released in some error cases

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Platform:
x86
Windows XP
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: gavin.reaney, Assigned: gavin.reaney)

References

Details

(Keywords: fixed1.8.0.10, fixed1.8.1.1, Whiteboard: [need testcase])

Attachments

(1 file)

Proposed fix
(deleted), patch
brendan
: review+
mconnor
: approval1.8.1.1+
jay
: approval1.8.0.10+
Details | Diff | Splinter Review
JS_strtod doesn't release the dtoa lock if the function exits via the 'nomem' label. Similar problems exist in JS_dtobasestr. This could lead to a hang in OOM conditions.
Attached patch Proposed fix (deleted) — — Splinter Review
Added calls to release the lock. Also fixed a few cases where word0(rv) = x was being used instead of set_word0(rv, x)
Please ask Brendan for a review.
Comment on attachment 242864 [details] [diff] [review] Proposed fix Ready for review now (was just double checking the code first etc).
Attachment #242864 - Flags: review?(brendan)
Assignee: general → gavin
Comment on attachment 242864 [details] [diff] [review] Proposed fix r=me, thanks. /be
Attachment #242864 - Flags: review?(brendan)
Attachment #242864 - Flags: review+
Attachment #242864 - Flags: approval1.8.1.1?
Blocks: js1.7src
Flags: blocking1.8.1.1?
will take on branch, doesn't immediately seem to be a blocker
Flags: wanted1.8.1.x+
Flags: blocking1.8.1.1?
Flags: blocking1.8.1.1-
Comment on attachment 242864 [details] [diff] [review] Proposed fix a=mconnor on behalf of drivers for 1.8.1.1 checkin
Attachment #242864 - Flags: approval1.8.1.1? → approval1.8.1.1+
Fixed on trunk and 1.8 branch: Checking in jsdtoa.c; /cvsroot/mozilla/js/src/jsdtoa.c,v <-- jsdtoa.c new revision: 3.38; previous revision: 3.37 done Checking in jsdtoa.c; /cvsroot/mozilla/js/src/jsdtoa.c,v <-- jsdtoa.c new revision: 3.33.2.2; previous revision: 3.33.2.1 done /be
Status: NEW → RESOLVED
Closed: 18 years ago
Keywords: fixed1.8.1.1
Resolution: --- → FIXED
Flags: in-testsuite-
Flags: wanted1.8.1.x+
Blocks: 351739
Attachment #242864 - Flags: approval1.8.1.2?
The bug exists on 1.8.0 branch as is.
Flags: blocking1.8.1.2?
Comment on attachment 242864 [details] [diff] [review] Proposed fix Igor: Did you mean to ask for 1.8.0.10 approval? Just want to make sure before approving this patch. Let us know. Thanks!
Attachment #242864 - Flags: approval1.8.1.2? → approval1.8.0.10?
Comment on attachment 242864 [details] [diff] [review] Proposed fix The patch for the bug applies as-is for 1.8.0 branch.
Comment on attachment 242864 [details] [diff] [review] Proposed fix Approved for 1.8.0 branch, a=jay for drivers. Igor says this patch can land there as is. Thanks!
Attachment #242864 - Flags: approval1.8.0.10? → approval1.8.0.10+
Whiteboard: [checkin needed (1.8.0 branch)]
I committed the patch from comment 1 to MOZILLA_1_8_0_BRANCH: Checking in jsdtoa.c; /cvsroot/mozilla/js/src/jsdtoa.c,v <-- jsdtoa.c new revision: 3.33.10.2; previous revision: 3.33.10.1 done
Keywords: fixed1.8.0.10
Whiteboard: [checkin needed (1.8.0 branch)]
Flags: blocking1.8.1.2? → blocking1.8.0.10+
Whiteboard: [need testcase]
gavin, can you verify this fix against the 1_8_0 branch? It sounds like we could use a developer's to check this unless you know of a easy way for QA to verify. Thanks.
(In reply to comment #13) > gavin, can you verify this fix against the 1_8_0 branch? It sounds like we > could use a developer's to check this unless you know of a easy way for QA to > verify. Thanks. > I've run a build from the 1.8.0 branch and stepped through the code in the affected functions using the visual studio debugger. Locks are correctly released when handling error conditions.
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: