Bug 16725 – Memory leak in kjs_binding.cpp

• Status: RESOLVED
• Resolution: INVALID
• Component: JavaScriptCore

[Adam Treat]

I did not check trunk, but it appears the safari-3 branch of kjs_binding.cpp is leaking memory here:

void ScriptInterpreter::putDOMNodeForDocument(Document* document, Node* node, JSNode* wrapper)
{
    ADD_WRAPPER(wrapper);
    if (!document) {
        domObjects().set(node, wrapper);
        return;
    }
    NodeMap* documentDict = domNodesPerDocument().get(document);
    if (!documentDict) {
        documentDict = new NodeMap;
        domNodesPerDocument().set(document, documentDict);
    }
    documentDict->set(node, wrapper);
}

Where does the documentDict get deleted?

Here is what valgrind has to say about this:

==7746== 8,216 (24 direct, 8,192 indirect) bytes in 1 blocks are definitely lost in loss record 378 of 507
==7746==    at 0x4022F0C: operator new(unsigned) (vg_replace_malloc.c:163)
==7746==    by 0x4736883: KJS::ScriptInterpreter::putDOMNodeForDocument(WebCore::Document*, WebCore::Node*, WebCore::JSNode*) (kjs_binding.cpp:189)
==7746==    by 0x4730568: WebCore::toJS(KJS::ExecState*, WTF::PassRefPtr<WebCore::Node>) (JSNodeCustom.cpp:216)
==7746==    by 0x4C8BE4B: WebCore::JSHTMLDocumentPrototypeFunction::callAsFunction(KJS::ExecState*, KJS::JSObject*, KJS::List const&) (JSHTMLDocument.cpp:444)
==7746==    by 0x47013E3: KJS::JSObject::call(KJS::ExecState*, KJS::JSObject*, KJS::List const&) (object.cpp:94)
==7746==    by 0x46E5F25: KJS::FunctionCallDotNode::evaluate(KJS::ExecState*) (nodes.cpp:791)
==7746==    by 0x46DC166: KJS::AssignExprNode::evaluate(KJS::ExecState*) (nodes.cpp:1579)
==7746==    by 0x46E2538: KJS::VarDeclNode::evaluate(KJS::ExecState*) (nodes.cpp:1637)
==7746==    by 0x46E22A2: KJS::VarDeclListNode::evaluate(KJS::ExecState*) (nodes.cpp:1684)
==7746==    by 0x46E20D6: KJS::VarStatementNode::execute(KJS::ExecState*) (nodes.cpp:1708)
==7746==    by 0x46DECCD: KJS::SourceElementsNode::execute(KJS::ExecState*) (nodes.cpp:2569)
==7746==    by 0x46DCFDB: KJS::BlockNode::execute(KJS::ExecState*) (nodes.cpp:1740)

[Geoffrey Garen]

void ScriptInterpreter::forgetAllDOMNodesForDocument(Document* document)
{
    ASSERT(document);
    NodeMap* map = domNodesPerDocument().take(document);
    if (!map)
        return;
    removeWrappers(*map);
    delete map; // <-- BINGO
}

[Adam Treat]

Are you sure this is being called for all nodes?  Valgrind says this is definitely lost...