148171 - js_FinishDtoa() not called unless JS_THREADSAFE is set

Status: VERIFIED FIXED

(Core :: JavaScript Engine, defect)

Description

[Steven Cole]

This is a follow on to bug 85267: js_FinishDtoa() is not called unless
JS_THREADSAFE is set.  (Since js_FinishDtoa() is only called inside
js_CleanupLocks(), which is only available for JS_THREADSAFE builds.)

Since js_FinishDtoa() now does memory cleanup as well, we should call it even
for builds with JS_THREADSAFE not set.

Patch coming up.

--scole

Comment 1

[Steven Cole]

Attachment: Proposed Patch

Note that this patch *does not* remove the call to js_FinishDtoa from
js_CleanupLocks, but adds one in JS_Shutdown for non threadsafe builds.  I'm
not entirely sure whether the call in js_CleanupLocks should be removed or not.


--scole

Comment 2

[Steven Cole]

Attachment: Revised patch

I missed an unprototyped function warning in the prior patch; this fixes that.

--scole

Comment 3

[Steven Cole]

Nominating for JS1.5.

Comment 4

[Phil Schwartau]

Need r=, sr= to get this patch in for JS 1.5 -

Comment 5

[Kenton Hanson (gone)]

Comment on attachment 85624 [details] [diff] [review]
Revised patch

r=khanson

Comment 6

[Steven Cole]

>Ping<

Just need an sr= on this one...

--scole

Comment 7

[Mike Shaver (:shaver emeritus)]

Comment on attachment 85624 [details] [diff] [review]
Revised patch

sr=shaver

Comment 8

[Steven Cole]

Woohoo! Thanks, shaver... Kenton, can you check this in for me, please?  --scole

Comment 9

[Phil Schwartau]

I think we want this on the branch as well, to keep the trunk
and branch in sync; so drivers will have to be emailed also -

Comment 10

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 85624 [details] [diff] [review]
Revised patch

Approved for 1.0 branch; please change mozilla1.0.1+ to fixed1.0.1 when checked
in.  Please check in ASAP

Comment 11

[timeless]

checked in.

Comment 12

[David Baron :dbaron: (⌚️UTC-5, no longer working on Mozilla)]

The jslock.c change broke the Mac builds.

Comment 13

[Steven Cole]

Oh, drat.  timeless:  Add a 
   #include "jsdtoa.h"
in the includes section of jslock.c to get this working again.  (Can you do a
mac build to try it out first, though?)

--scole

Comment 14

[Phil Schwartau]

Patch verified on trunk and MOZILLA_1_0_BRANCH.