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

Fix memory leak in message list #4139

Merged
merged 2 commits into from
Sep 14, 2020
Merged

Conversation

danxuliu
Copy link
Member

Besides the memory leak fixed here, which was always reproducible, there seems to be another one related to the message list that happens sometimes (and apparently with Chromium, but not with Firefox 🤷). After a lot of time and effort I have not been able to reproduce it reliably nor find why it happens either (it seems to be related to XMLHttpRequests/Axios and getNewMessages(), but other than that I am still clueless), so I give up for now :-(

How to test

  • In Firefox, open a conversation with lots of messages
  • Open the developer tools
  • Take a memory snapshot; its size will be around 100MB
  • Start and leave a call five times in a row
  • In a different tab, open about:memory and trigger a garbage collection (Free memory -> GC)
  • In the previous tab, take another memory snapshot

Result with this pull request

The size of the new memory snapshot will be around the same value as the previous one

Result without this pull request

The size of the new memory snapshot will be much higher (around 300MB)

This will be needed to unsubscribe the handlers, as it was not possible
to refer to the arrow functions from other parts of the code.

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
This fixes a memory leak of the message list as well as any other
component that the message list is part of (like the chat view due to
the parent component reference of Vue).

Signed-off-by: Daniel Calviño Sánchez <danxuliu@gmail.com>
@danxuliu
Copy link
Member Author

/backport to stable19

@danxuliu
Copy link
Member Author

/backport to stable18

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants