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

.css('marginRight') - memory leak #1795

Closed
mgol opened this Issue Oct 21, 2014 · 2 comments

Comments

Projects
None yet
3 participants
@mgol
Member

mgol commented Oct 21, 2014

Originally reported by flexphperia at: http://bugs.jquery.com/ticket/15235

I have found memory leak in reliableMarginRight function.

Memory leak is created when you call $element.css('marginRight')

JsFiddle:  http://fiddle.jshell.net/z3jo9k1q/show/light/

Test case:

  1. open above address in Chrome,
  2. open developer tools, make Heap snapshot,
  3. click on button in jsfiddle example, red element will be removed
  4. make another heap snapshot with browser tools
  5. compare snapshots, you will find one HtmlDivElement that is detached from DOM but keeping reference in reliableMarginRight function

Fix: Adding this, after 5665 line in jQuery uncompressed code will fix the problem:

docElem.removeChild( container );
div.removeChild( marginDiv ); //this is new line

Issue reported for jQuery 2.1.1

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Oct 21, 2014

Member

Comment author: flexphperia

Here's direct link to jsFiddle :  http://fiddle.jshell.net/z3jo9k1q/
Click RUN before testing for memory leaks in Chrome.

Is there any chances to fix this bugi in next release? It's quite important.

Member

mgol commented Oct 21, 2014

Comment author: flexphperia

Here's direct link to jsFiddle :  http://fiddle.jshell.net/z3jo9k1q/
Click RUN before testing for memory leaks in Chrome.

Is there any chances to fix this bugi in next release? It's quite important.

@markelog markelog added the CSS label Nov 2, 2014

@dmethvin dmethvin self-assigned this Dec 1, 2014

@dmethvin

This comment has been minimized.

Show comment
Hide comment
@dmethvin

dmethvin Dec 1, 2014

Member

Pretty clearly a leak since div is in an outer scope. I can't unit test it but I'll add the removal.

Member

dmethvin commented Dec 1, 2014

Pretty clearly a leak since div is in an outer scope. I can't unit test it but I'll add the removal.

dmethvin added a commit to dmethvin/jquery that referenced this issue Dec 1, 2014

dmethvin added a commit that referenced this issue Dec 2, 2014

CSS: Clean up memory leak in reliableMarginRight
Fixes gh-1795
Closes gh-1893

Thanks for the report flexphperia!
(cherry picked from commit 7d15b4d)

Conflicts:
	src/css/support.js

@dmethvin dmethvin closed this in 7d15b4d Dec 2, 2014

@dmethvin dmethvin added this to the 3.0.0 milestone Dec 8, 2014

@dmethvin dmethvin added the Bug label Dec 8, 2014

dmethvin added a commit that referenced this issue Dec 11, 2014

CSS: Clean up memory leak in reliableMarginRight
Fixes gh-1795
Closes gh-1893

Thanks for the report flexphperia!

dmethvin added a commit that referenced this issue Dec 11, 2014

CSS: Clean up memory leak in reliableMarginRight
Fixes gh-1795
Closes gh-1893

Thanks for the report flexphperia!
(cherry picked from commit 7d15b4d)

Conflicts:
	src/css/support.js

dmethvin added a commit that referenced this issue Dec 11, 2014

CSS: Clean up memory leak in reliableMarginRight
(cherry-picked from 7d15b4d)

Fixes gh-1795
Closes gh-1893

Thanks for the report flexphperia!

dmethvin added a commit that referenced this issue Dec 11, 2014

CSS: Clean up memory leak in reliableMarginRight
(cherry-picked from fa70df6)

Fixes gh-1795
Closes gh-1893

Thanks for the report flexphperia!
(cherry picked from commit 7d15b4d)

Conflicts:
	src/css/support.js

@dmethvin dmethvin modified the milestones: 1.11.2/2.1.2, 3.0.0 Dec 15, 2014

dmethvin added a commit that referenced this issue Nov 10, 2015

CSS: Clean up memory leak in reliableMarginRight
Fixes gh-1795
Closes gh-1893

Thanks for the report flexphperia!

@cssmagic cssmagic referenced this issue May 18, 2016

Open

jQuery #5

@lock lock bot locked as resolved and limited conversation to collaborators Jun 20, 2018

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