Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JQuery 1.11.3 Memory Leak on IE8 #2685

Closed
arvstract opened this issue Nov 3, 2015 · 12 comments
Closed

JQuery 1.11.3 Memory Leak on IE8 #2685

arvstract opened this issue Nov 3, 2015 · 12 comments

Comments

@arvstract
Copy link

Hi Guys,

We're developing a website using asp.net and JQuery v.1.11.3. It has frameset and frames (althought it is unsupported in HTML5). Our target clients use IE8 on Windows XP.

We encountered memory leak in IE8 everytime a page is being rendered by the frame element. I have also read several articles which states that there are memory leaks when using frames in IE8.

So how do I confirm it as a bug? Well first I tried our website on other browsers such as Chrome and Firefox and it seems that memory leak doesn't exists. And then I get back to IE8 and downgrading the JQuery version step by step, I started at 1.11.3, 1.11.2, 1.11.1 and then I stopped at 1.11.0, this is the most current version that the memory leak DOESNT exists.

I haven't bump yet into any articles that has the same problem that I encountered, hope you can shed a light regarding this.

Thank you and more power to the JQuery team!

@dmethvin
Copy link
Member

dmethvin commented Nov 3, 2015

For reference, this is a continuation of a forum post. @arvstract would you be able to take a look at the diff I posted there and identify possible sources of the leak? It would help us a lot!

@arvstract
Copy link
Author

hi Dave, to be honest I'm not that familiar on how the internal code of JQuery works so I tried reporting the bug as what I observe in my unit test and hope that the JQuery team can use this as a reference. As of now I'm currently reviewing the URL that you gave me and doing the best that I can to understand the code. And also, can you share some ideas on how I can help on isolating the code that causes the memory leak? Thank you!

@dmethvin
Copy link
Member

dmethvin commented Nov 3, 2015

@arvstract Sure, we'd love to get some help from you on this bug since you're seeing it. I mentioned this change in the full differences between versions, if you put that line back does the leak go away? It may have been lost in a cherry-pick.

@dmethvin
Copy link
Member

dmethvin commented Nov 4, 2015

Looking at this a bit more I'm pretty sure we lost a few IE8 leak-plugs during the move to make support checks lazy. There should be a div = body = container = null; right here shouldn't there? Most likely we need them in all the broken-out feature detects for attributes, manipulation, css, and effects, we already have them in event and data.

@mgol
Copy link
Member

mgol commented Nov 4, 2015

It's unrelated to making the support tests lazy, I kept all those checks in badcd1b. The line was removed in b96522a by @gibson042, @markelog claimed in #1518 (comment) this bug existed only in unpatched IE6 versions. Was that false?

@dmethvin
Copy link
Member

dmethvin commented Nov 4, 2015

I'm not sure ... I see I was summoned but didn't answer, sorry.

There is a bug with IE6 SP1 that caused a memory leak with event handlers that was fixed with IE6 SP2 (in 2004!) and several were fixed in IE7, but there is at least one that applies through IE8. I am not sure whether the patterns we have in there trigger the leak, but it wouldn't surprise me if it did. @markelog do you recall more about it? I mean give me a break here it's been a decade! 😄

@markelog
Copy link
Member

markelog commented Nov 4, 2015

I'm not sure, but it seems, in that comment, i might have referenced this update, which is 2007.

Although Microsoft says memory issues should have been solved in IE8, there is evidence of resurfacing new but different leaks in it. It seems as not our case though, since assigning everything to null (as described in that article) wouldn't help.

What @arvstract is describing, sounds like an iframe issue. I couldn't find any mentions of update with which such fix was released.

But, it shouldn't have matter, because as mentioned in original discussion, if issue is present, it should leak only small amount of memory.

So unless this app is not constantly removing and adding iframes with jquery inside it should work fine. But if app, in fact, does do that, then i wouldn't advice adding this fix back in, to support such unusual use case.

@markelog
Copy link
Member

markelog commented Nov 4, 2015

but there is at least one that applies through IE8

Sounds as not our case, since it is about expandos

@dmethvin
Copy link
Member

dmethvin commented Nov 4, 2015

I agree, I didn't see how it could apply. At this point I think we are waiting for @arvstract to provide some more information about the case where it appears to leak.

@arvstract
Copy link
Author

Hi @markelog , our website is composed of frameset which is structure more or less like this:

<frameset framespacing="0" border="0" frameborder="NO" cols="*" rows="60,*">
    <frame src="header.aspx" name="__topFrame" scrolling="no" noresize="noresize" />
    <frameset id="__leftFrameSetter" framespacing="0" border="0" frameborder="NO" cols="207,*">
        <frame src="navigation.aspx" name="__leftFrame" scrolling="auto" noresize="noresize" />
        <frame src="<URL's of webforms goes here>" name="__mainFrame" />
    </frameset>
</frameset>

If you're navigating constantly, the "__mainFrame" will keep on loading pages with JQuery in it, which causes the memory to build up (but not being released) which is not existent on v1.11.0, upon "refreshing" the page the memory clears up.

If this is what you mean by "unless this app is not constantly removing and adding iframes with jquery inside it" then yes, we are constantly removing and adding iframes with jquery in it.

Hi @dmethvin , I'm truly sorry that I'm still unable to provider any information about the bug that I encounter because we have been busy fixing bugs on our website this past few days, we've decided to downgrade to JQuery v1.11.0 for now. And sorry to tell you that I still cannot guarantee yet when I can review and pinpoint on where the issue might possibly occur.

@dmethvin
Copy link
Member

dmethvin commented Nov 5, 2015

I can imagine that IE8 suffers from memory leaks in that situation because the frameset page is never unloaded and that is when the leak would be cleaned up. But it could also be that I have a vivid imagination, so that's why it would be good to get more info about it and whether putting those null assignments back solves the problem. We don't like to change code without a good test case. In a non-frameset page any leak from the support tests should be minor since they only happen once.

@markelog
Copy link
Member

markelog commented Dec 2, 2015

Closing for now, if you will provide more info, we can investigate further

@markelog markelog closed this as completed Dec 2, 2015
@lock lock bot locked as resolved and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

5 participants