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

[Browser Client] jQuery memory leak #1910

Closed
jvilk opened this Issue Aug 16, 2017 · 2 comments

Comments

Projects
None yet
2 participants
@jvilk
Contributor

jvilk commented Aug 16, 2017

Problem

Mailpile's JavaScript UI currently suffers from this jQuery memory leak. The cause is repeated calls to $(document).ready(), which seem to happen every time Mailpipe updates the UI.

The fix should be present in the next jQuery release.

Severity

I wrote a script that performed the following steps in a loop in a single session:

  • For the top four messages in the inbox:
    • Click on the message to view it.
    • Click on the inbox to return to the message listing.
  • Take a heap snapshot

I then graphed Mailpile's JavaScript heap size over each iteration of that loop:

mailpile

The leak is noticeable, and may add up over a long session.

Here's a graph of Mailpile's memory usage with the memory leak fixed:

mailpile_fixed

Possible Fixes

  • Update to the newest jQuery when it is released
  • Manually apply the small patch from the PR
  • Avoid calling $(document).ready(function() {}) so often. I am not sure if this behavior is present in Mailpile or one of its dependency.

@jvilk jvilk changed the title from Client-side jQuery memory leak to [Browser Client] jQuery memory leak Aug 17, 2017

@BjarniRunar

This comment has been minimized.

Member

BjarniRunar commented Aug 19, 2017

Thank you for the excellent bug report. I agree with your first two suggestions; the third is a bit harder since we rely on being able to call $(document).ready(...) from multiple locations as a simple way to keep the code loosely coupled; each fragment can have its own initialization logic without needing to coordinate with the rest of the page.

@BjarniRunar

This comment has been minimized.

Member

BjarniRunar commented Mar 8, 2018

I am going to close this; it should get addressed when we update to the latest versions of everything (right before freezing 1.0), which is issue: #1800

@BjarniRunar BjarniRunar closed this Mar 8, 2018

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