Last Comment Bug 357392 - jsdtoa.c - locks not released in some error cases
: jsdtoa.c - locks not released in some error cases
Status: RESOLVED FIXED
[need testcase]
: fixed1.8.0.10, fixed1.8.1.1
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: x86 Windows XP
: -- normal (vote)
: ---
Assigned To: Gavin Reaney
:
:
Mentors:
Depends on:
Blocks: 351739 js1.7src
  Show dependency treegraph
 
Reported: 2006-10-20 04:43 PDT by Gavin Reaney
Modified: 2007-02-09 15:22 PST (History)
6 users (show)
mconnor: blocking1.8.1.1-
dveditz: blocking1.8.0.10+
bob: in‑testsuite-
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Proposed fix (1.82 KB, patch)
2006-10-20 04:46 PDT, Gavin Reaney
brendan: review+
mconnor: approval1.8.1.1+
jaymoz: approval1.8.0.10+
Details | Diff | Splinter Review

Description Gavin Reaney 2006-10-20 04:43:36 PDT
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.
Comment 1 Gavin Reaney 2006-10-20 04:46:40 PDT
Created attachment 242864 [details] [diff] [review]
Proposed fix

Added calls to release the lock. Also fixed a few cases where word0(rv) = x was being used instead of set_word0(rv, x)
Comment 2 Bob Clary [:bc:] 2006-10-20 07:48:13 PDT
Please ask Brendan for a review.
Comment 3 Gavin Reaney 2006-10-20 07:56:59 PDT
Comment on attachment 242864 [details] [diff] [review]
Proposed fix

Ready for review now (was just double checking the code first etc).
Comment 4 Brendan Eich [:brendan] 2006-10-20 08:16:48 PDT
Comment on attachment 242864 [details] [diff] [review]
Proposed fix

r=me, thanks.

/be
Comment 5 Mike Connor [:mconnor] 2006-11-22 08:16:29 PST
will take on branch, doesn't immediately seem to be a blocker
Comment 6 Mike Connor [:mconnor] 2006-11-22 08:17:19 PST
Comment on attachment 242864 [details] [diff] [review]
Proposed fix

a=mconnor on behalf of drivers for 1.8.1.1 checkin
Comment 7 Brendan Eich [:brendan] 2006-11-22 14:36:14 PST
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
Comment 8 Igor Bukanov 2007-01-17 16:01:12 PST
The bug exists on 1.8.0 branch as is.
Comment 9 Jay Patel [:jay] 2007-01-18 14:26:11 PST
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!
Comment 10 Igor Bukanov 2007-01-18 14:41:40 PST
Comment on attachment 242864 [details] [diff] [review]
Proposed fix

The patch for the bug applies as-is for 1.8.0 branch.
Comment 11 Jay Patel [:jay] 2007-01-18 14:46:29 PST
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!
Comment 12 Igor Bukanov 2007-01-18 14:56:01 PST
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
Comment 13 Tony Chung [:tchung] 2007-02-09 12:26:08 PST
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.
Comment 14 Gavin Reaney 2007-02-09 15:22:00 PST
(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.

Note You need to log in before you can comment on or make changes to this bug.