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

IE9 memory leaks #513

Closed
jfroffice opened this Issue Feb 29, 2012 · 20 comments

Comments

Projects
None yet
7 participants
@jfroffice

jfroffice commented Feb 29, 2012

You can find the test page here:
http://jsfiddle.net/d2UEB/39/show/

The code :
http://jsfiddle.net/d2UEB/39

This Bug does not occur when using default custom & minimize version.

@paulirish

This comment has been minimized.

Show comment
Hide comment
@paulirish

paulirish Mar 4, 2012

Member

@ryanseddon verified...

So issue #513 is kinda a problem, I can confirm that the dev file does have a memory leak the ram usage increases by about 15-20mb per refresh so I guess uglifyjs must be fixing the issue when it's built.

Member

paulirish commented Mar 4, 2012

@ryanseddon verified...

So issue #513 is kinda a problem, I can confirm that the dev file does have a memory leak the ram usage increases by about 15-20mb per refresh so I guess uglifyjs must be fixing the issue when it's built.

@jfroffice

This comment has been minimized.

Show comment
Hide comment
@jfroffice

jfroffice Mar 4, 2012

Thank you for your reply.

I though that it could be a special test that might cause the memory leak. But for now, I do not find the real cause. As you say, UglifyJS seems to fix the leakage.

jfroffice commented Mar 4, 2012

Thank you for your reply.

I though that it could be a special test that might cause the memory leak. But for now, I do not find the real cause. As you say, UglifyJS seems to fix the leakage.

@ryanseddon

This comment has been minimized.

Show comment
Hide comment
@ryanseddon

ryanseddon Mar 5, 2012

Member

Ok so this is interesting, I dug a little deeper to see what could be causing the dev file to leak compared to the your build. So the first bit I tried was removing the geolocation test, and that stops the memory leaks for me in IE9 win7. It'll hover around 25mb and won't creep up.

Can you confirm this behaviour too.

Full dev build: http://jsfiddle.net/ryanseddon/d2UEB/42/show/ (this creeps up around 3-5mb per refresh)

Full dev build with no geolocation: http://jsfiddle.net/ryanseddon/d2UEB/43/show/

I killed the iexplorer process before testing each fiddle and only had one tab open at a time.

Member

ryanseddon commented Mar 5, 2012

Ok so this is interesting, I dug a little deeper to see what could be causing the dev file to leak compared to the your build. So the first bit I tried was removing the geolocation test, and that stops the memory leaks for me in IE9 win7. It'll hover around 25mb and won't creep up.

Can you confirm this behaviour too.

Full dev build: http://jsfiddle.net/ryanseddon/d2UEB/42/show/ (this creeps up around 3-5mb per refresh)

Full dev build with no geolocation: http://jsfiddle.net/ryanseddon/d2UEB/43/show/

I killed the iexplorer process before testing each fiddle and only had one tab open at a time.

@paulirish

This comment has been minimized.

Show comment
Hide comment
@paulirish

paulirish Mar 5, 2012

Member

whoaaaaa.. that's a bug alright.

wow.

Member

paulirish commented Mar 5, 2012

whoaaaaa.. that's a bug alright.

wow.

@jfroffice

This comment has been minimized.

Show comment
Hide comment
@jfroffice

jfroffice Mar 5, 2012

@ryanseddon ... I also confirm this behaviour.

Full dev build: http://jsfiddle.net/ryanseddon/d2UEB/42/show/ (this creeps up around 3-5mb per refresh)
Full dev build with no geolocation: http://jsfiddle.net/ryanseddon/d2UEB/43/show/ (stay around 25mb and won't creep up)

I test with IE version: 9.0.8112.16421

A quick diff shows that :

/**
     * geolocation tests for the new Geolocation API specification.
     *   This test is a standards compliant-only test; for more complete
     *   testing, including a Google Gears fallback, please see:
     *   code.google.com/p/geo-location-javascript/
     * or view a fallback solution using google's geo API:
     *   gist.github.com/366184
     */
    tests['geolocation'] = function() {
             return !!navigator.geolocation;
    };

geolocation is buggy in IE9. Whoaaaa...

jfroffice commented Mar 5, 2012

@ryanseddon ... I also confirm this behaviour.

Full dev build: http://jsfiddle.net/ryanseddon/d2UEB/42/show/ (this creeps up around 3-5mb per refresh)
Full dev build with no geolocation: http://jsfiddle.net/ryanseddon/d2UEB/43/show/ (stay around 25mb and won't creep up)

I test with IE version: 9.0.8112.16421

A quick diff shows that :

/**
     * geolocation tests for the new Geolocation API specification.
     *   This test is a standards compliant-only test; for more complete
     *   testing, including a Google Gears fallback, please see:
     *   code.google.com/p/geo-location-javascript/
     * or view a fallback solution using google's geo API:
     *   gist.github.com/366184
     */
    tests['geolocation'] = function() {
             return !!navigator.geolocation;
    };

geolocation is buggy in IE9. Whoaaaa...

@leodutra

This comment has been minimized.

Show comment
Hide comment
@leodutra

leodutra Mar 5, 2012

Humm... I'm going to my work now. But someone should report it for MS.
This is ridiculous...

leodutra commented Mar 5, 2012

Humm... I'm going to my work now. But someone should report it for MS.
This is ridiculous...

@ryanseddon

This comment has been minimized.

Show comment
Hide comment
Member

ryanseddon commented Mar 6, 2012

@leodutra

This comment has been minimized.

Show comment
Hide comment
@leodutra

leodutra Mar 6, 2012

Good job dude.
The bad part is... I'm still working and they want me to login to see a freak bug.
Hail to MS, always conquering the magic F*ck Off Lands.

leodutra commented Mar 6, 2012

Good job dude.
The bad part is... I'm still working and they want me to login to see a freak bug.
Hail to MS, always conquering the magic F*ck Off Lands.

@paulirish

This comment has been minimized.

Show comment
Hide comment
@paulirish

paulirish Mar 6, 2012

Member

in summary this is all that causes the bug:

!!navigator.geolocation;

So this will cause the leak for anyone checking geolocation in IE9. This is not isolated to Modernizr.

FYI!


screenshot of the IE Connect filing because who wants to log into that mess?!

Member

paulirish commented Mar 6, 2012

in summary this is all that causes the bug:

!!navigator.geolocation;

So this will cause the leak for anyone checking geolocation in IE9. This is not isolated to Modernizr.

FYI!


screenshot of the IE Connect filing because who wants to log into that mess?!

@leodutra

This comment has been minimized.

Show comment
Hide comment
@leodutra

leodutra Mar 6, 2012

Wow! Thanks Paul.
Only the best ninjas of the JavaScript can save us from "MicroShift".

And congrats for Modernizr!

leodutra commented Mar 6, 2012

Wow! Thanks Paul.
Only the best ninjas of the JavaScript can save us from "MicroShift".

And congrats for Modernizr!

@paulirish

This comment has been minimized.

Show comment
Hide comment
@paulirish

paulirish Mar 6, 2012

Member

It was all Ryan. 👲

Member

paulirish commented Mar 6, 2012

It was all Ryan. 👲

@staabm

This comment has been minimized.

Show comment
Hide comment
@staabm

staabm commented Mar 6, 2012

/cc

@ryanseddon

This comment has been minimized.

Show comment
Hide comment
@ryanseddon

ryanseddon Mar 7, 2012

Member

Got confirmation from MS that it can be reproduced by them and strangely in IE10PP5, PP4 was fine for me. IE9 was the only one leaking on my end.

Member

ryanseddon commented Mar 7, 2012

Got confirmation from MS that it can be reproduced by them and strangely in IE10PP5, PP4 was fine for me. IE9 was the only one leaking on my end.

@paulirish

This comment has been minimized.

Show comment
Hide comment
@paulirish

paulirish Apr 16, 2012

Member

So we just changed the geolocation check to avoid a webkit page caching issue.

see recent commit from @josh

its now 'geolocation' in navigator .. i wonder if that triggers the mem leak here.

Member

paulirish commented Apr 16, 2012

So we just changed the geolocation check to avoid a webkit page caching issue.

see recent commit from @josh

its now 'geolocation' in navigator .. i wonder if that triggers the mem leak here.

@paulirish

This comment has been minimized.

Show comment
Hide comment
@paulirish

paulirish Apr 16, 2012

Member

just tested with http://jsfiddle.net/d2UEB/46/show/ and things look great! no leak with josh's new test. boom

Member

paulirish commented Apr 16, 2012

just tested with http://jsfiddle.net/d2UEB/46/show/ and things look great! no leak with josh's new test. boom

@ryanseddon

This comment has been minimized.

Show comment
Hide comment
@ryanseddon

ryanseddon Apr 16, 2012

Member

I though I was seeing a massively reduced leak still but it seems to of stabilised around 33mb on a fresh start with the above testcase running. Boom indeed.

Member

ryanseddon commented Apr 16, 2012

I though I was seeing a massively reduced leak still but it seems to of stabilised around 33mb on a fresh start with the above testcase running. Boom indeed.

@josh

This comment has been minimized.

Show comment
Hide comment
@josh

josh Apr 16, 2012

Contributor

🤘

Contributor

josh commented Apr 16, 2012

🤘

@jfroffice

This comment has been minimized.

Show comment
Hide comment
@jfroffice

jfroffice Apr 16, 2012

works for me.

great news !

jfroffice commented Apr 16, 2012

works for me.

great news !

@jfroffice jfroffice closed this Apr 16, 2012

@ryanseddon

This comment has been minimized.

Show comment
Hide comment
@ryanseddon

ryanseddon Jun 6, 2012

Member

Got a response that this issue has been fixed in IE10PP6

Member

ryanseddon commented Jun 6, 2012

Got a response that this issue has been fixed in IE10PP6

@MarkPieszak

This comment has been minimized.

Show comment
Hide comment
@MarkPieszak

MarkPieszak Oct 16, 2012

Your the man Paul, had this exact issue today (I'm using 1.7), changed the return statement and the memory leak is no more! Thanks again.

MarkPieszak commented Oct 16, 2012

Your the man Paul, had this exact issue today (I'm using 1.7), changed the return statement and the memory leak is no more! Thanks again.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment