Opened 10 years ago

Closed 10 years ago

Last modified 10 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 10 years ago by Artur Delura

Owner: set to Artur Delura
Status: newassigned

comment:2 Changed 10 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.

Sad fact is that main CKEditor instance is not released which keep all references. I'm gonna create issue for that.

Changes in branch:t/12261

Version 1, edited 10 years ago by Artur Delura (previous) (next) (diff)

comment:3 Changed 10 years ago by Artur Delura

Status: assignedreview

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

Milestone: CKEditor 4.4.4CKEditor 4.4.5

comment:5 Changed 10 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 10 years ago by Artur Delura

Note: See TracTickets for help on using tickets.
© 2003 – 2022, CKSource sp. z o.o. sp.k. All rights reserved. | Terms of use | Privacy policy