25911 - Leaking JSContexts

Status: RESOLVED FIXED

(Core :: XPConnect, defect, P3)

Description

[Patrick C. Beard]

You are creating XPCContext objects in XPCJSRuntime::SyncXPCContextList(), and 
adding them to a JSContext2XPCContextMap (XPCJSRuntime::mContextMap), but 
evidently not cleaning up the contents of the map in 
XPCJSRuntime::~XPCJSRuntime().

Comment 1

[Patrick C. Beard]

Attachment: Annotated leak report

Comment 2

[Patrick C. Beard]

Attachment: HTML version of annotated leak report.

Comment 3

[John Bandhauer]

No.

The management of XPCContext objects is working correctly. SyncXPCContextList 
uses KillDeadContextsCB which deletes unneeded XPCContext objects an removed 
them from the hashtable using:

    // this XPCContext represents a dead JSContext - delete it
    delete xpcc;
    return HT_ENUMERATE_REMOVE;

This can be verified by running a xpconnect in an embedding that does not leak 
the underlying JSContexts; e.g. xpcshell running xpctest_echo.js

No, the real problem is that mozilla.exe leaks JSContexts and/or does not notify 
xpconnect to resync in all places where JSContexts are deleted.

I have leak detection code in XPCJSRuntime::~XPCJSRuntime() I just added a block 
to iterate all JSContexts and report the count:

#ifdef DEBUG_jband
    {
    // count the total JSContexts in use
    JSContext* iter = nsnull;
    int count = 0;
    while(JS_ContextIterator(mJSRuntime, &iter))
        count ++;
    printf("deleting XPCJSRuntime with %d total live JSContexts\n", count);        
    }
#endif

Running the checked in bloaty tests I see:

deleting XPCJSRuntime with 3 total live JSContexts
deleting XPCJSRuntime with 3 live JSContexts known by xpconnect
deleting XPCJSRuntime with 2 live xpcwrappednativeclasses
...and...
the report lines show that one nsJSContext is leaked. 

There must be other JSContexts created in the system. 

I can explore further.

[changing bug title from...
  [MLK] Leaking XPCContexts
...to...
  [MLK] Leaking JSContexts

Comment 4

[Patrick C. Beard]

I only report what I see. My attachment shows that there's an unaccounted for 
XPCContext, that ISN'T referred to by any other leaked object. This is a GC-based 
leak detector, so if the object were being referenced by any other leaked object, 
I would see it.

Comment 5

[John Bandhauer]

Sure. The xpconnect shutdown code is whacking the map that references the 
XPCContext objects and yes I agree that when the map gets whacked there is no 
reason to not delete any remaining XPCContexts. I can add that code. This is a 
shutdown only issue. XPCContext objects correctly come and go during the life of 
the app.

Still, the underlying problem is really that we are leaking JSContexts. 

Comment 6

[John Bandhauer]

These JSContexts are leaked because nsJSContexts are leaked by the cycle 
described in http://bugzilla.mozilla.org/show_bug.cgi?id=28570

Comment 7

[Bruce Mitchener]

Is this fixed then?

Comment 8

[John Bandhauer]

I believe this is fixed.