597654 - TM: shutdown leak after jsapi-tests/testTrap_gc

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Igor Bukanov]

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]

Attachment: 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]

Nominating for branches as the leak exists in all tracing jit releases

Comment 3

[Igor Bukanov]

http://hg.mozilla.org/tracemonkey/rev/dfcf5611ce02

Comment 4

[Daniel Veditz [:dveditz]]

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]

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 6

[Robert Sayre]

http://hg.mozilla.org/mozilla-central/rev/dfcf5611ce02

Comment 7

[Daniel Veditz [:dveditz]]

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]]

Comment on attachment 476509 [details] [diff] [review]
v1

missed the 1.9.2.11/1.9.1.14 releases

Comment 9

[Daniel Veditz [:dveditz]]

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]

My preference is to land this rather safe fix.

Comment 12

[Igor Bukanov]

http://hg.mozilla.org/releases/mozilla-1.9.2/rev/320791250625

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/95fb9d4c1dcc