Memory Leaks when dynamically loading

[rid00z]

Found some JQuery Mobile widgets attach closures to the window and document object and JQuery Mobile was not cleaning them up.

So code below would result in a memory leak.

$('#reload').click(function () {

    $('#content').empty();

    $.ajax({
        url: 'injectHtml.htm',
        async: false,
        success: function (data) { $('#content').append(data).trigger("create"); }
    });

});

[toddparker]

@rid00z - Thanks for reporting. Mind creating a test page illustrating the issue? Template: http://jsbin.com/uqenom/edit

[rid00z]

Hi

That should reproduce it. http://jsbin.com/uqenom/22/edit

[gabrielschulhof]

OK, so .empty() ends up calling _destroy() on widget instances that have this. We need to implement this for all widgets that attach global handlers.

[rid00z]

Yeh I ended up having to patch the widgets myself. It's working in my app,
I just need to submit patch to core.

On Thursday, June 21, 2012, Gabriel |Nix| Schulhof wrote:

OK, so .empty() ends up calling _destroy() on widget instances that have
this. We need to implement this for all widgets that attach global handlers.

---

Reply to this email directly or view it on GitHub:
#4419 (comment)

Michael Ridland | ThinkSmart Digital
Managing Director
P. 0404 865 350
E. michael@thinksmartdigital.com.au
W. www.thinksmartdigital.com.au
T. www.twitter.com/rid00z
L. au.linkedin.com/in/michaelridland

http://au.linkedin.com/in/michaelridland

[gabrielschulhof]

@rid00z

Thanks for the PR!

There are a few more leaky places you could look at, if you'd like:

• fixedToolbar: It does a lot of $el.closest( ".ui-page" ).bind( ... ) meaning that, if you remove it from the page, the binding stays attached to the page.
• navbar also attaches handlers to the closest page, and so it would leak similarly to fixedToolbar
• loader does a bunch of $window.bind(...) and $window.unbind(...), but I can't tell from a cursory glance whether or not it leaves some bindings in place if you were to destroy the widget.

Fixing these should allow us to close this issue.

[gabrielschulhof]

The new widget factory has a this.bindings object where we can list all the elements to which we bind things and, as long as they're bound with a namespace, they will be removed.

[gabrielschulhof]

... so instead of the problematic $( document ).bind( "eventname", function() {} ) way of doing things, now you can simply this._on( document, { eventname: function() {} ) or even simpler this._on( document, { eventname: "_memberFunction" } ) and the handler will be namespaced uniquely to the widget instance and it will be removed upon _destroy().

[toddparker]

@rid00z - thanks for your help. If you'd like to help out, hop on #jquerymobile-dev on freenode and we'll find some uses for your talents.

[jzaefferer]

Saw this in the meeting notes: "the new widget factory has this._on() which nicely prevents memory leaks caused by stale attached event handlers, however, we should implement _destroy() on our widgets, because handlers attached to elements introduced during widget creation need not be removed if the elements are removed." - as long as you're using _on to bind events, you don't need to implement _destroy yourself - am I missing something?

[gabrielschulhof]

@jzaefferer

Our widgets need to implement _destroy() so as to roll back changes they made during _create() (such as e.g. new DOM elements), but they can rely on the _destroy() inside $.Widget to remove the bindings.

[jzaefferer]

Are there still widgets that don't clean up after themselves? If so, maybe individual tickets for each affected widget might help get this resolved.

[jaspermdegroot]

Yes, this is still work in progress. We have a generic ticket for adding the missing _destroy() method (#5293) but I agree it's better if we have separate tickets for each widget.

@arschmitz @gabrielschulhof - Can you open tickets for widgets that aren't done yet and close #5293? If this issue is completely resolved when all widgets have _destroy() methods it can be closed as well to avoid duplication.

Thanks!

Update: See See #5293 (comment). Closing this ticket as duplicate. Closing this ticket as duplicate.