The default bug view has changed. See this FAQ.

ExecutablePool::release can mask leaky references




JavaScript Engine
6 years ago
6 years ago


(Reporter: bhackett, Assigned: njn)


Firefox Tracking Flags

(Not tracked)


(Whiteboard: fixed-in-tracemonkey)


(1 attachment)

If an executable pool is in the m_smallPools list on ExecutableAllocator destruction, then release is called with willDestroy == true.  The JS_ASSERT_IF sets the refcount to one rather than testing it against one, so leaky references are not uncovered.

    void release(bool willDestroy = false)
        JS_ASSERT(m_refCount != 0);
        JS_ASSERT_IF(willDestroy, m_refCount = 1);
        if (--m_refCount == 0) {

Comment 1

6 years ago
Oh geez, =/== fail.  My bad.

How did you find this -- code inspection, or something else?

Comment 2

6 years ago
This is why I prefer the "const == var" form for comparisons, BTW... but it's not standard Mozilla/SpiderMonkey practice :(
(In reply to comment #1)
> Oh geez, =/== fail.  My bad.
> How did you find this -- code inspection, or something else?

I was working on bug 656753 (reference leak in the TI branch) and trying to figure out why a testcase I wrote exposing the bug didn't assert.  Took some printfs and gdb watching before I noticed this, very sneaky assignment.

Don't know how often we have problems like these, but it would be nifty to have a pseudo-policy of no assignments at all in conditionals, then a tool could pick up violations super easy.


6 years ago
Depends on: 654820

Comment 4

6 years ago
Created attachment 532544 [details] [diff] [review]

This fixes the =/== confusion and also disables the failing assertion, so that the tinderbox won't go orange.  It can be re-enabled when bug 654820 is fixed.
Assignee: general → nnethercote
Attachment #532544 - Flags: review?(bhackett1024)
Attachment #532544 - Flags: review?(bhackett1024) → review+

Comment 5

6 years ago
Whiteboard: fixed-in-tracemonkey

Comment 6

6 years ago
Follow up patch that disables the assertion because it's triggering in xpcshell tests:

I've added the details of the failure to bug 654820 which is almost certainly related.
cdleary-bot mozilla-central merge info:
Last Resolved: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.