Memory leak

[minhtranite]

Is this a bug report or feature request? (choose one)

🐞 Bug report

💻 Version of CKEditor

<script src="https://cdn.ckeditor.com/ckeditor5/11.1.1/balloon/ckeditor.js"></script>

📋 Steps to reproduce

1. Create a editor instance
2. Destroy it

✅ Expected result

Memory not increment

❎ Actual result

Memory increment each time create and destroy

📃 Other details that might be useful

• Codepend: https://codepen.io/minhtranite/pen/Xybegm?editors=1010
• Screenshot video: https://drive.google.com/file/d/17Hqae0B60V8BR0Cq0CjiVJL7-cgc6R7S/view?usp=sharing

[oleq]

I also confirm that:

After the first destruction

After the second destruction

cc @Reinmar

[Reinmar]

This is a regression because we tested for that about 2 years ago and there were no leaks at all.

[oleq]

about 2 years ago

I think that's why we should make it a part of every major release.

[Reinmar]

Yes. We should have a regression test for that. As a general rule – since we have a bug, we must have a test. Ideally – automated. AFAIR there was some API available for making snapshots and accessing such performance information. Even if it's in just one browser, let's have a test for that.

[minhtranite]

Sorry! What is iteration 22 milestone mean ? It not assign due date so I want to ask when this issue can be fixed ? I'm using it on production app so It will be good if you can fix it soon.

[jodator]

@minhtranite Hi the good news is that we identified what was the problem and it will be fixed soon.

The iteration 22 is a current one so the next official release should have this fix included.

[jodator]

@Reinmar & @pomek: Some technical question about Travis integration. The tests uses Google Chrome run with additional flag in order to expose garbage collector for tests. The tests itself are generic one that runs create/destroy couple of times and compares used JS heap size after first run and after the last.

After some fiddling with them locally I've identified that 1 MB is sufficient to both minimize false positives and to actually catch a memory leaks automatically. Unfortunately memory measurements are repeatable to some extent. To minimize errors the garbage collecting step is necessary.

Also I think that running memory leak tests only for Chrome is sufficient to detect them.

The question is how to enable them on CI? Note: there will be dedicated manual test pages with instructions on how to check memory usage.

Note that memory leak tests are long running - I've added dome timeouts before crate/destroy steps in order to have everything run and allow garbage collector to work. Also as memory test aren't 100% reproducible with used memory the failed test is retried to eliminate false positives. The test itself take about 6-7 seconds to complete due to timeouts.

We can either:

• expose garbage collector for all the tests run with Chrome (add a flag to current Chrome configuration)
• or create dedicated step which will run only memory related tests with dedicated Chrome configuration

ps.: The tests are skipped when the window.gc() is not available.

Edit: It looks like the window.gc() is blocking so no timeouts are needed and tests runs within normal timeout. So the test can be run with others and we can modify the default Chrome configuration.

[jodator]

After some fiddling with them locally I've identified that 1 MB is sufficient to both minimize false positives

There was one more component causing memory leaks. Probably we're going to be safe with 0 as a test there.

[minhtranite]

Thank you guys!