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

The inspector causes memory leaks #52

Open
jodator opened this issue Jun 7, 2019 · 6 comments
Open

The inspector causes memory leaks #52

jodator opened this issue Jun 7, 2019 · 6 comments
Labels
domain:dx squad:core Issue to be handled by the Core team. type:bug

Comments

@jodator
Copy link
Contributor

jodator commented Jun 7, 2019

There are at least two problems:

  1. Logging to a console editor instance - until the console is cleared the editor will remain no available for garbage collector (below only logs the editor without instantiating the inspector):
    Peek 2019-06-07 14-53

  2. The inspector itself hold some references to editors and thus they are not cleared (the console was cleared):
    Selection_039

@oleq
Copy link
Member

oleq commented Jun 14, 2019

Yeah, you're probably right because this is something we ignored to speed up development. It could be a tough one to debug (React).

OTOH it's not that critical because you can disable the inspector and then check out the memory leaks, right?

@jodator
Copy link
Contributor Author

jodator commented Jun 14, 2019

Yes you could - but there's no OFF switch for our manual test to not load the inspector at all for manual memory leak related tests.

@oleq
Copy link
Member

oleq commented Jun 14, 2019

If you don't expose it as Window.editor, it won't load automatically.

@oleq
Copy link
Member

oleq commented Jun 14, 2019

BTW, wouldn't this be enough #42?

@jodator
Copy link
Contributor Author

jodator commented Jun 14, 2019

There was more text but I've checked #42 and I see that the inspector listens to destroy event and then it detaches itself 👍.

The only thing that probably (I didn't tested #42 for leaks) would be to get read logging the editor instance to the console. I'm not sure if this is really useful. The better would be logging the editor instance to the console similarly as the commands can be logged to the console using >_ button.

@jodator
Copy link
Contributor Author

jodator commented Jun 17, 2019

@oleq As your request from ckeditor/ckeditor5#1814 (comment) unfortunately the changes from PR: #46 does not help. I'm worried that it is either React itself or there are still some cleanups to be done:

Peek 2019-06-17 11-08

On the above screen cast I do:

  1. Create/destroy the editor couple of times.
  2. Clear console.
  3. Run the CKEditorInspector.destroy()

Unfortunately somethings retains references to the the editor.
Selection_044

@Mgsy Mgsy added this to the next milestone Sep 16, 2019
@jodator jodator modified the milestones: next, nice-to-have Aug 26, 2020
@Reinmar Reinmar added squad:core Issue to be handled by the Core team. and removed squad:ux labels Sep 27, 2021
@pomek pomek removed this from the nice-to-have milestone Feb 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:dx squad:core Issue to be handled by the Core team. type:bug
Projects
None yet
Development

No branches or pull requests

6 participants