Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#12261 closed Bug (fixed)

Filter has to be destroyed and removed from CKEDITOR.filter.instances on editor destroy

Reported by: Piotrek Koszuliński Owned by: Artur Delura
Priority: Normal Milestone: CKEditor 4.4.5
Component: General Version: 4.3
Keywords: Cc:

Description

This issue is related to #11010 (it's a sub case of it).

When filter is created it's registered in CKEDITOR.filter.instances. Unfortunately, we haven't got time to implement any mechanism cleaning this on editor destroy. Therefore, when system destroys and initialises editors many time, the pile of filters grows and each filter consumes pretty a lot of memory.

The solution should be simple. From my tests it seems that this will be enough:

  • Implement filter.destroy() that will for safety delete the private object (this._) which keeps a lot of stuff and will remove editor from CKEDITOR.filter.instances.
  • Call editor.filter.destroy() and delete editor.filter and editor.activeFilter on editor#destroy

We can also track some other big objects that are retained after editor destroy and try deleting references we know off. Sometimes it won't help, but some times it may.

Based on http://ckeditor.com/forums/CKEditor/CKEditor-multiple-instance-memory-leak

Change History (6)

comment:1 Changed 4 years ago by Artur Delura

Owner: set to Artur Delura
Status: newassigned

comment:2 Changed 4 years ago by Artur Delura

For leak tests I used replacebycode.html sample and I commented initial creating instance (just to make memory snapshot before any initialization). Code which I used for creating and destroying instance looks as follow:

CKEDITOR.replace( 'editor1' );
// Wait some period of time.
CKEDITOR.instances.editor1.destroy();

At the beggining memory usage was 8.9 mb. First create and destroy CKEditor make extra 3mb (after changes +2.8) But it's normal because first initialization make server requests for plugins etc.

Second create and destroy CKEditor make extra .7mb (after changes +.5mb). Most resource consuming was Commands and ui.Buttons but I managed to release them with CKEditor as well as filter instances.

Sad fact is that main CKEditor instance is not released on destroy. And it keeps all references. I'm gonna create issue for that.

Changes in branch:t/12261

Last edited 4 years ago by Artur Delura (previous) (diff)

comment:3 Changed 4 years ago by Artur Delura

Status: assignedreview

comment:4 Changed 4 years ago by Piotrek Koszuliński

Milestone: CKEditor 4.4.4CKEditor 4.4.5

comment:5 Changed 4 years ago by Piotrek Koszuliński

Resolution: fixed
Status: reviewclosed

I pushed additional commit, because I thought that it will be safer to cut filter instance into pieces on destroy. It might be referenced in some additional place (plugin e.g.) and these 3 properties I delete are pretty big.

Merged to master with git:731f5ad.

I also checked that deleting dataProcessor should be rather simple, but commands are tough - they are referenced from many places. I haven't seen buttons as main threat, but it's definitely worth to free them. Please report next, more generic issue so you'll be able to work on buttons, UI in general, commands, data processor and perhaps plugins too.

comment:6 Changed 4 years ago by Artur Delura

Note: See TracTickets for help on using tickets.
© 2003 – 2017 CKSource – Frederico Knabben. All rights reserved. | Terms of use | Privacy policy