Bug Tracker

Opened 7 years ago

Closed 5 years ago

#12171 closed bug (patchwelcome)

Event binding on disconnected element causes memory leak in IE7

Reported by: johnhunter Owned by:
Priority: low Milestone: 1.next
Component: event Version: 1.8rc1
Keywords: Cc:
Blocked by: Blocking:

Description

Creating a disconnected element and then binding an event handler to it causes all subsequent bind calls to leak their elements. When the page is unloaded references remain for every element that has a binding.

Browser: IE7
Test environment: sIEve-0.0.8 (http://home.wanadoo.nl/jsrosman/)
Test case: http://jsfiddle.net/johnhunter/3nFL7/ (will need to run this outside of jsfiddle to avoid side effects in IE7)

Steps to reproduce:

  1. Save test case to local server.
  2. Load in sIEve.
  3. Reload page (or use auto-refresh)

Expected result:
No leaked elements detected.

Actual result:
The tt element is leaked along with a div (one of jQuery's support test elements)

Can also replicate this with the following:

  • jquery-1.7.2
  • jquery-1.7.1
  • jquery-1.6.1 (with additional leaking support elements)

so this is not new to 1.8.

This issue may also effect IE8 but I am unable to confirm at present.

Change History (16)

comment:1 Changed 7 years ago by dmethvin

#12172 is a duplicate of this ticket.

comment:2 Changed 7 years ago by dmethvin

The test case boils down to this:

    $('<div/>').bind('click', function(){});
    $('tt').bind('click', function(){});

You said the div that was leaked was one of jQuery's support test elements, are you sure it's not the div from the test case? Does a test case with just one of the lines above leak as well? I can't try it myself at the moment.

comment:3 in reply to:  2 Changed 7 years ago by johnhunter

Replying to dmethvin:

You said the div that was leaked was one of jQuery's support test elements, are you sure it's not the div from the test case? Does a test case with just one of the lines above leak as well? I can't try it myself at the moment.

Retested the disconnected div changed to a code tag.

   $('<code/>').bind('click', function(){});
   $('tt').bind('click', function(){});

The following elements leak:

  • the bound tt element
  • the disconnected code element
  • the html element
  • a div element (a generated support element?)

Testing this with jquery1.7.2 leaks only the tt and code elements.

If I remove the line for the tt bind then I get 3 leaking elements (the tt is no longer leaked). If I remove the line for the code bind then there is no leak as would be expected.

Hope that clarifies things.

comment:4 Changed 7 years ago by dmethvin

Status: newopen

Okay, this needs more investigation.

comment:5 Changed 7 years ago by mikesherov

Component: unfiledevent

comment:6 Changed 6 years ago by timmywil

Priority: undecidedlow

comment:7 Changed 6 years ago by dmethvin

#12949 is a duplicate of this ticket.

comment:8 Changed 6 years ago by Tomislavt

Good morning, is there any known workaround or patch for this problem? I too have it, on ever page reload using latest jQuery and jQuery UI around 5-15 mb is leaked on IE7

comment:9 Changed 6 years ago by peter.nystrom

I created a wokraround in jQuery UI 1.8.19 by cleaning up the dpDiv variable in the Datepicker when the window is unloaded. Unbind it and set to null. Have not tested it on later versions. This change removed the memory leak that occured by just including jQuery and jQuery UI on any page.

comment:10 Changed 6 years ago by johnhunter

Bear in mind the original case was unrelated to JqueryUI. Still unresolved.

comment:11 Changed 6 years ago by dmethvin

Still unresolved.

Yes, thats what the (open bug) means at the top of the page. If you have the time please take a look, we'd welcome the help. But please don't bump the ticket.

comment:12 in reply to:  9 Changed 6 years ago by acetrike@…

Replying to peter.nystrom:

I created a wokraround in jQuery UI 1.8.19 by cleaning up the dpDiv variable in the Datepicker when the window is unloaded. Unbind it and set to null.

Can you please provide the code for this?!

comment:13 Changed 6 years ago by ctoestreich

I was able to fix this in IE6 by adding window unload event handling for undelegating the bound events that were creating circular reference. This change was added directly to the end of the function Datepicker(){...}.

Clearing of the object references is just fluff, the real meat of the memory leak problem can be turned on and off by simply commenting in and out the line that does the undelegate.

function Datepicker() {
        ...

        var self = this;
        $(window).unload(function(){
            var selector = 'button, .ui-datepicker-prev, .ui-datepicker-next, .ui-datepicker-calendar td a';
            //this line if the magic
            self.dpDiv.undelegate(selector, 'mouseover mouseout');
            self._dialogInst = self.dpDiv = null;
        });
}

Sorry for pasting the entire jquery ui lib in the js window, but here is the fix in action:

http://jsfiddle.net/ctoestreich/JFETB/

As a bit of clarification, the method to add the module datepicker to jQuery is fired on page load and even when not specifically using datepickers on your page, simply the act of including the library would cause the problem as the orphaned object delegates were getting created on page load by:

 $.datepicker = new Datepicker(); // singleton instance

This is called by the module on every load which in turn calls the object construction and adds the dpDiv and its event delegates that get orphaned due to circular references and being unattached to the dom in any way.

Cheers!

Last edited 6 years ago by ctoestreich (previous) (diff)

comment:14 Changed 6 years ago by peter.nystrom

My solution was similar to the one above, I created this function in the Datepicker that was called on window unload:

	cleanup: function(target) {
		this.dpDiv.unbind();
		this.dpDiv = null;
	},

comment:15 Changed 6 years ago by m_gol

Milestone: None1.next

comment:16 Changed 5 years ago by dmethvin

Resolution: patchwelcome
Status: openclosed

We don't have the resources or inclination to fix this, and it's not a fatal error. Given the point we're at with IE6/7 support we're unlikely to land a fix unless it was only one or two lines, but if someone has such a fix we could consider it.

Note: See TracTickets for help on using tickets.