Closed
Bug 597654
Opened 13 years ago
Closed 13 years ago
TM: shutdown leak after jsapi-tests/testTrap_gc
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
People
(Reporter: igor, Assigned: igor)
Details
(Whiteboard: fixed-in-tracemonkey)
Attachments
(1 file)
834 bytes,
patch
|
gal
:
review+
dveditz
:
approval1.9.2.11-
christian
:
approval1.9.2.13+
dveditz
:
approval1.9.1.14-
christian
:
approval1.9.1.16+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•13 years ago
|
||
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.
Assignee: general → igor
Attachment #476509 -
Flags: review?(gal)
Updated•13 years ago
|
Attachment #476509 -
Flags: review?(gal) → review+
Assignee | ||
Comment 2•13 years ago
|
||
Nominating for branches as the leak exists in all tracing jit releases
blocking1.9.1: --- → ?
blocking1.9.2: --- → ?
blocking2.0: --- → ?
Assignee | ||
Comment 3•13 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/dfcf5611ce02
Whiteboard: fixed-in-tracemonkey
Comment 4•13 years ago
|
||
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.
blocking1.9.1: ? → needed
blocking1.9.2: ? → needed
status1.9.1:
--- → wanted
status1.9.2:
--- → wanted
Assignee | ||
Comment 5•13 years ago
|
||
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.
Attachment #476509 -
Flags: approval1.9.2.11?
Attachment #476509 -
Flags: approval1.9.1.14?
Comment 6•13 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/dfcf5611ce02
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Comment 7•13 years ago
|
||
Comment on attachment 476509 [details] [diff] [review] v1 Approved for 1.9.2.11 and 1.9.1.14, a=dveditz for release-drivers
Attachment #476509 -
Flags: approval1.9.2.11?
Attachment #476509 -
Flags: approval1.9.2.11+
Attachment #476509 -
Flags: approval1.9.1.14?
Attachment #476509 -
Flags: approval1.9.1.14+
Comment 8•13 years ago
|
||
Comment on attachment 476509 [details] [diff] [review] v1 missed the 1.9.2.11/1.9.1.14 releases
Attachment #476509 -
Flags: approval1.9.2.11-
Attachment #476509 -
Flags: approval1.9.2.11+
Attachment #476509 -
Flags: approval1.9.1.14-
Attachment #476509 -
Flags: approval1.9.1.14+
Comment 9•13 years ago
|
||
Do you still want to land this on the 1.9.2/1.9.1 branches or should we wontfix this?
Assignee | ||
Comment 10•13 years ago
|
||
My preference is to land this rather safe fix.
Attachment #476509 -
Flags: approval1.9.2.12+
Attachment #476509 -
Flags: approval1.9.1.15+
Assignee | ||
Comment 12•13 years ago
|
||
http://hg.mozilla.org/releases/mozilla-1.9.2/rev/320791250625 http://hg.mozilla.org/releases/mozilla-1.9.1/rev/95fb9d4c1dcc
Updated•13 years ago
|
blocking2.0: ? → betaN+
You need to log in
before you can comment on or make changes to this bug.
Description
•