DOM and memory leaks when destroying an editor instance #2469

alekseybobkov · 2015-04-25T01:48:18Z

Ace editor requires a better handling of object references, DOM references and event handlers in the destroy() method. Although the method partially cleans up the memory, unfortunately, it still leaves some references inside the object: circular references between the editor and internal objects and closures, references to the DOM elements and unbound events. It creates problems with using the editor in large single-page applications, where multiple editor instances can be created and removed on a page during the user session. The biggest issue is the detached DOM trees, which take much memory and slows down the application.

I created a demo here: http://jsfiddle.net/hLge6j38/3/ Run it in Google Chrome and review the heap snapshot in Profiler after removing the editor. On the Contentment tab you will find a detached DOM tree (74 entries), which root is referred by the "container" item in the editor.

The Summary tab shows circular references between the editor and env objects:

Cleaning the editor's properties from outside reveals more issues, in particular, the textarea.ace_text-input element is referenced in the context for all closures defined in TextInput class: https://github.com/ajaxorg/ace-builds/blob/master/src/ace.js#L1891

The Summary tab displays other objects belonging to Ace after deleting the editor instance.

At the moment I can't find direct indications that unbound event handlers cause issues, but I'm pretty sure they do and will be revealed after cleaning up the references contained in variables.

Related: #344

nightwing · 2015-04-25T22:48:15Z

related to #2432
with the latest build editor.destory should is fixed, and when testing with the following code i do not see any leaking objects

<script src="https://ajaxorg.github.io/ace-builds/src/ace.js"></script>
<p><button onclick="toggleEditor()">Toggle editor</button></p>
<div id="editor" style="height: 100px;">function foo(items) {
    var x = "All this is syntax highlighted";
    return x;
}</div>
<script>
    var editor
    function toggleEditor() {
        if (!editor) {
            var root = document.getElementById('editor')
            root.parentNode.insertBefore(root.cloneNode(true), root)
            editor = ace.edit(root);
            editor.setTheme("ace/theme/monokai");
            editor.session.setMode("ace/mode/javascript");
        } else {
            editor.destroy();
            var el = editor.container;
            el.parentNode.removeChild(el);
            editor = null;
        }
    }
    toggleEditor()
</script>

alekseybobkov · 2015-04-26T00:36:55Z

Thanks for the update! The situation seems to be much better now. I played with your code (slightly modified to load different themes) and I see that GC removes all Editor instances but one (for some reason a single instance always exists).

But it still leaves the detached DOM tree:

The number of detached trees doesn't depend on how many editor instances I create and destroy and that's a good indicator. But even a single tree could cause issues in a large application - it could prevent a larger tree from being removed.

From what I see in the Profiler reports, the DOM element is referred by "container" property of the editor. If I set that property to NULL in my script, the Profiler shows a reference in the "renderer" property. If I set it to NULL as well, it complaints to the VirtualRenderer object:

The code that I used for testing, for your reference. In the code I create and remove the root DIV element on each iteration.

<!DOCTYPE html>
<html lang="en">
    <head>
        <meta charset="utf-8">
        <script src="https://ajaxorg.github.io/ace-builds/src/ace.js"></script>
    </head>
    <body>
        <p><button onclick="toggleEditor()">Toggle editor</button></p>

        <div id="container"></div>
        <script>
            var editor,
                counter = 0

            function toggleEditor() {
                if (!editor) {
                    var root = document.createElement('div');
                    root.style.height = '100px';
                    root.setAttribute('id', 'editor');
                    root.textContent = 'function foo(items) {var x = "All this is syntax highlighted"; return x;';

                    document.getElementById('container').appendChild(root);

                    editor = ace.edit(root);

                    if (counter++ % 2)
                        editor.setTheme("ace/theme/monokai");
                    else
                        editor.setTheme("ace/theme/clouds");

                    editor.session.setMode("ace/mode/javascript");
                } else {
                    editor.destroy();
                    var el = editor.container;
                    el.parentNode.removeChild(el);

                    editor.container = null;
                    editor.renderer = null;

                    editor = null;
                }
            }
            toggleEditor()
        </script>
    </body>
</html>

nightwing · 2015-04-26T10:48:27Z

You are right, ref to the first created editor was retained in a closure. #2470 fixes that.

ghost · 2019-10-08T08:33:18Z

@nightwing
we noticed the following when using editor with scroll. The same code used above is used to test it.

when loading the editor with less content. We can find the Editor reference.
On destroy, the editor seems to have got cleared properly.
when adding extra lines , scroll functionality is added to the editor.
On destroy, The editor reference still seems to be present.

On further checking we found that, the virtualrenderer was still having reference to editor once scroll events were bonded.

removing the scroll events using jquery .off() before destroying fixed this.

I think while destroying the editor, we have to remove the events to lose the reference they have to the editor.

nightwing · 2019-10-08T08:54:56Z

@soundar78 could you show the code you used to add remove events, or better the code you used for testing? That would help me to understand what is causing the leak.

ghost · 2019-10-08T09:36:29Z

For reproducing i used the same code from this comment

nightwing · 2019-10-08T12:25:08Z

this appears to be a regression in chrome, the same issue happens with the following code when typing into textarea, without typing the memory doesn't increase, and on firefox the memory doesn't increase either

<script>
    function Editor(element) {
        this.setValue = function(v) { this.value = v.split("\n") }
        this.getValue = function() { return this.value.join("\n") }
        this.destroy = function() {}
        
        var self = this;
        this.container = element
        this.setValue(element.textContent)
        
        var text = document.createElement("textarea")
        element.appendChild(text)
        element.addEventListener("mousedown", function() {
            console.log(this, self, element)
            text.focus()
        })
    }
    var ace = {
        edit: function(element) {
            return new Editor(element)
        }
    }
    var editor
    function toggleEditor() {
        if (!editor) {
            var root = document.getElementById('editor')
            root.parentNode.insertBefore(root.cloneNode(true), root)
            editor = ace.edit(root, {
                theme: "ace/theme/monokai",
                mode: "ace/mode/javascript",
            });
            var val = editor.getValue();
            
            editor.setValue(
                new Array(10000).join(val), -1
            )
        } else {
            editor.destroy();
            var el = editor.container;
            el.parentNode.removeChild(el);
            editor = null;
        }
    }
    toggleEditor()
</script>

nightwing · 2019-10-09T08:28:23Z

@soundar78 how did you use jquery.off to fix the issue? I have reported the chrome bug at https://bugs.chromium.org/p/chromium/issues/detail?id=1012636, but your solution would be useful until it is fixed.

ghost · 2019-10-10T07:14:48Z

I actually used something similar to the below code to fix the memory leak in our app. But it did not help in fixing the memory leak with the below example.

<script src="https://ajaxorg.github.io/ace-builds/src/ace.js"></script>
<script src="https://code.jquery.com/jquery-1.10.1.js"></script>
<p><button onclick="toggleEditor()">Toggle editor</button></p>
<div id="editor" style="height: 100px;">function foo(items) {
    var x = "All this is syntax highlighted";
    return x;
}</div>
<script>
    var editor;
    function toggleEditor() {
        if (!editor) {
            var root = document.getElementById('editor')
            root.parentNode.insertBefore(root.cloneNode(true), root)
            editor = ace.edit(root);
            editor.setTheme("ace/theme/monokai");
            editor.session.setMode("ace/mode/javascript");
        } else {
            editor.renderer.scrollBarV.off('scroll');
            editor.renderer.scrollBarH.off('scroll');
            $( editor.textInput.getElement() ).off();
            editor.destroy();
            var el = editor.container;
            el.parentNode.removeChild(el);
            editor = null;
        }
    }
    toggleEditor();
</script>

ghost · 2019-10-10T07:22:29Z

this appears to be a regression in chrome, the same issue happens with the following code when typing into textarea, without typing the memory doesn't increase, and on firefox the memory doesn't increase either

<script>
    function Editor(element) {
        this.setValue = function(v) { this.value = v.split("\n") }
        this.getValue = function() { return this.value.join("\n") }
        this.destroy = function() {}
        
        var self = this;
        this.container = element
        this.setValue(element.textContent)
        
        var text = document.createElement("textarea")
        element.appendChild(text)
        element.addEventListener("mousedown", function() {
            console.log(this, self, element)
            text.focus()
        })
    }
    var ace = {
        edit: function(element) {
            return new Editor(element)
        }
    }
    var editor
    function toggleEditor() {
        if (!editor) {
            var root = document.getElementById('editor')
            root.parentNode.insertBefore(root.cloneNode(true), root)
            editor = ace.edit(root, {
                theme: "ace/theme/monokai",
                mode: "ace/mode/javascript",
            });
            var val = editor.getValue();
            
            editor.setValue(
                new Array(10000).join(val), -1
            )
        } else {
            editor.destroy();
            var el = editor.container;
            el.parentNode.removeChild(el);
            editor = null;
        }
    }
    toggleEditor()
</script>

but i was actually able to remove the editor reference event after typing in the textarea using the below code.

<script src="https://ajaxorg.github.io/ace-builds/src/ace.js"></script>
<p><button onclick="toggleEditor()">Toggle editor</button></p>
<div id="editor" style="height: 100px;">function foo(items) {
    var x = "All this is syntax highlighted";
    return x;
}</div>
<script>
    function Editor(element) {
        this.setValue = function(v) { this.value = v.split("\n") }
        this.getValue = function() { return this.value.join("\n") }
        this.destroy = function() { element.removeEventListener("mousedown", mouseDownHandler ); }
        
        this.container = element
        this.setValue(element.textContent)
        
        var mouseDownHandler = function()
        {
            console.log(this, self, element)
            text.focus()
        }

        var text = document.createElement("textarea")
        element.appendChild(text)
        element.addEventListener("mousedown", mouseDownHandler );
    }
    var ace = {
        edit: function(element) {
            return new Editor(element)
        }
    }
    var editor
    function toggleEditor() {
        if (!editor) {
            var root = document.getElementById('editor')
            root.parentNode.insertBefore(root.cloneNode(true), root)
            editor = ace.edit(root, {
                theme: "ace/theme/monokai",
                mode: "ace/mode/javascript",
            });
            var val = editor.getValue();
            
            editor.setValue(
                new Array(10000).join(val), -1
            )
        } else {
            editor.destroy();
            var el = editor.container;
            el.parentNode.removeChild(el);
            editor = null;
        }
    }
    toggleEditor()
</script>

alekseybobkov changed the title ~~DOM and memory leaks when destroying an editor~~ DOM and memory leaks when destroying an editor instance Apr 25, 2015

nightwing mentioned this issue Apr 26, 2015

Fix memory leak #2470

Merged

nightwing closed this as completed in a842dcd May 6, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DOM and memory leaks when destroying an editor instance #2469

DOM and memory leaks when destroying an editor instance #2469

alekseybobkov commented Apr 25, 2015

nightwing commented Apr 25, 2015

alekseybobkov commented Apr 26, 2015

nightwing commented Apr 26, 2015

ghost commented Oct 8, 2019

nightwing commented Oct 8, 2019

ghost commented Oct 8, 2019 •

edited by ghost

Loading

nightwing commented Oct 8, 2019

nightwing commented Oct 9, 2019

ghost commented Oct 10, 2019 •

edited by ghost

Loading

ghost commented Oct 10, 2019

DOM and memory leaks when destroying an editor instance #2469

DOM and memory leaks when destroying an editor instance #2469

Comments

alekseybobkov commented Apr 25, 2015

nightwing commented Apr 25, 2015

alekseybobkov commented Apr 26, 2015

nightwing commented Apr 26, 2015

ghost commented Oct 8, 2019

nightwing commented Oct 8, 2019

ghost commented Oct 8, 2019 • edited by ghost Loading

nightwing commented Oct 8, 2019

nightwing commented Oct 9, 2019

ghost commented Oct 10, 2019 • edited by ghost Loading

ghost commented Oct 10, 2019

ghost commented Oct 8, 2019 •

edited by ghost

Loading

ghost commented Oct 10, 2019 •

edited by ghost

Loading