Last Comment Bug 597654 - TM: shutdown leak after jsapi-tests/testTrap_gc
: TM: shutdown leak after jsapi-tests/testTrap_gc
Status: RESOLVED FIXED
fixed-in-tracemonkey
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: All All
: -- normal (vote)
: ---
Assigned To: Igor Bukanov
:
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-09-18 01:15 PDT by Igor Bukanov
Modified: 2010-12-10 08:35 PST (History)
3 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
betaN+
.13+
.13-fixed
needed
.16-fixed


Attachments
v1 (834 bytes, patch)
2010-09-18 02:44 PDT, Igor Bukanov
gal: review+
dveditz: approval1.9.2.11-
christian: approval1.9.2.13+
dveditz: approval1.9.1.14-
christian: approval1.9.1.16+
Details | Diff | Review

Description Igor Bukanov 2010-09-18 01:15:21 PDT
If in a debug build I add 
    JS_DumpHeap(cx, stderr, NULL, 0, NULL, size_t(-1), NULL); 
right after js_GC(cx, GC_LAST_CONTEXT); in js_DestroyContext, then running make checks gives:

testTrap_gc
TEST-PASS | testTrap_gc | ok
0xf5902000 global <no private>    via jitgcthing
0xf5902090 Object <no private>    via jitgcthing(0xf5902000 global).proto
0xf5901170 string __lookupSetter__ via jitgcthing(0xf5902000 global).proto(0xf5902090 Object).id
0xf5901160 string __lookupGetter__ via jitgcthing(0xf5902000 global).proto(0xf5902090 Object).id
0xf5901150 string __defineSetter__ via jitgcthing(0xf5902000 global).proto(0xf5902090 Object).id
0xf5901140 string __defineGetter__ via jitgcthing(0xf5902000 
...
global).Proxy(0xf5902b88 Proxy).createFunction
0xf590ea20 Function isTrapping    via jitgcthing(0xf5902000 global).Proxy(0xf5902b88 Proxy).isTrapping
0xf590ea80 Function fix           via jitgcthing(0xf5902000 global).Proxy(0xf5902b88 Proxy).fix

All leaks is through jitgcthing and only testTrap_gc gives such leak. All other tests passes with no output.
Comment 1 Igor Bukanov 2010-09-18 02:44:26 PDT
Created attachment 476509 [details] [diff] [review]
v1

The patch removes the TRACING_ENABLED(cx) check from PurgeScriptFragments. The check is bogus one as the tracing jit can be disabled after a script is jited when we still need to sweep dead script traces.
Comment 2 Igor Bukanov 2010-09-18 10:51:14 PDT
Nominating for branches as the leak exists in all tracing jit releases
Comment 4 Daniel Veditz [:dveditz] 2010-09-20 10:36:38 PDT
Not blocking, but we'll approve the patch. Is the one in the bug the right one for branches? If so please add an approval request.
Comment 5 Igor Bukanov 2010-09-20 12:29:01 PDT
Comment on attachment 476509 [details] [diff] [review]
v1

The patch requires a trivial merge to apply for branches to account for js_PurgeScriptFragments -> PurgeScriptFragments rename.
Comment 7 Daniel Veditz [:dveditz] 2010-09-22 10:59:33 PDT
Comment on attachment 476509 [details] [diff] [review]
v1

Approved for 1.9.2.11 and 1.9.1.14, a=dveditz for release-drivers
Comment 8 Daniel Veditz [:dveditz] 2010-10-04 10:12:26 PDT
Comment on attachment 476509 [details] [diff] [review]
v1

missed the 1.9.2.11/1.9.1.14 releases
Comment 9 Daniel Veditz [:dveditz] 2010-10-13 10:54:27 PDT
Do you still want to land this on the 1.9.2/1.9.1 branches or should we wontfix this?
Comment 10 Igor Bukanov 2010-10-13 12:13:05 PDT
My preference is to land this rather safe fix.

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