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

Memory leak in SearchBar component #99471

Closed
timroes opened this issue May 6, 2021 · 4 comments
Closed

Memory leak in SearchBar component #99471

timroes opened this issue May 6, 2021 · 4 comments
Assignees
Labels
bug Fixes for quality problems that affect the customer experience impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:small Small Level of Effort performance SharedUX/fix-it-week Bugs that have been groomed and queued up for the team's next fix it week

Comments

@timroes
Copy link
Contributor

timroes commented May 6, 2021

The SearchBar component currently has a severe memory leak. It creates ResizeObserver that it connects in it's componentDidMount method, but will never unobserve and disconnect that when the component gets unmounted, thus leading to the now unmounted (no longer used) ResizeObserver keeping around a reference to the DOM object it's watching and causing a potential very large detached DOM tree around. This will cause memory consumption and grow in detached (and not garbage collectable) DOM nodes every time this component unmounts. This is perceivable very well e.g. in Discover when you switch index patterns, which currently rerenders the page (thus unmounts and mount the search bar). Looking at the browsers performance tools you can see how every time the DOM node count and the JS heap will continue growing and never been cleaned up.

@timroes timroes added bug Fixes for quality problems that affect the customer experience performance Team:AppServices impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. labels May 6, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServices)

@Dosant Dosant added SharedUX/fix-it-week Bugs that have been groomed and queued up for the team's next fix it week loe:small Small Level of Effort labels May 6, 2021
@Dosant Dosant self-assigned this May 6, 2021
@Dosant Dosant added the triaged label May 6, 2021
@Dosant
Copy link
Contributor

Dosant commented May 10, 2021

@timroes, have you actually seen a leakage there from the search bar, or did you assume from reviewing code?
It looks like there is an unobserve

this.ro.unobserve(this.filterBarRef);

@timroes
Copy link
Contributor Author

timroes commented May 10, 2021

@Dosant I have actually seen the leakage and debugged it. That unobserve you're referring to is called in componentDidUpdate, but there is simply no componentWillUnmount present that will unobserve and destroy it. I validated that adding an unobserve and destroy in the componentWillUnmount actually resolved that described memory leak.

@Dosant
Copy link
Contributor

Dosant commented May 11, 2021

Fixed by #99603
thanks @timroes for brining this up

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Fixes for quality problems that affect the customer experience impact:high Addressing this issue will have a high level of impact on the quality/strength of our product. loe:small Small Level of Effort performance SharedUX/fix-it-week Bugs that have been groomed and queued up for the team's next fix it week
Projects
None yet
Development

No branches or pull requests

3 participants