Possible memory leak in autorefresh.js #7085

New issue

Open

opened 2024-01-18 13:07:13 +01:00 by marijnh · 1 comment

marijnh commented

2024-01-18 13:07:13 +01:00

(Migrated from gitlab.com)

Version:

codemirror: 5.65.12
Browser: Chrome 120.0.6099.217 (I believe this problem is browser-irrelevent)

Problem:

Code Analysis: As is shown in code below, when cm.display.wrapper.offsetHeight == 0 the autoRefresh addon register listeners of mouseup/keyup event on window, and continiously check the wrapper's offsetHeight. As soon as the offsetHeight check pass, these listeners will be unregistered.

Abnormal Situation: But under the situation that the offsetHeight of wrapper element is keeps 0 during the whole lifetime of a CodeMirror instance, the listener will never be released, causing the instance be referenced by listener on window, and never be gc'd.

My Case: In my circumstance, I have a dialog contains 2 CodeMirror instance, one of which is hidden by display: none. The dialog and its content is destroyed after closing, and re-create if opened again. So the hidden instance might never be rendered, and the wrapper's offsetHeight keeps 0(I'm using CodeMirror constructor to create instance, BTW).
I believe attempts to destroy instance have no use to this problem. For the stopListening function can only be called by check.

// addon/display/autorefresh.js
CodeMirror.defineOption("autoRefresh", false, function(cm, val) {
  if (cm.state.autoRefresh) {
    stopListening(cm, cm.state.autoRefresh)
    cm.state.autoRefresh = null
  }
  if (val && cm.display.wrapper.offsetHeight == 0)
    startListening(cm, cm.state.autoRefresh = {delay: val.delay || 250})
})

function startListening(cm, state) {
  function check() {

    // >>What if the offsetHeight is always 0 during the whole lifetime of CodeMirror instance?<<
    if (cm.display.wrapper.offsetHeight) {

      stopListening(cm, state)
      if (cm.display.lastWrapHeight != cm.display.wrapper.clientHeight)
        cm.refresh()
    } else {
      state.timeout = setTimeout(check, state.delay)
    }
  }
  state.timeout = setTimeout(check, state.delay)
  state.hurry = function() {
    clearTimeout(state.timeout)
    state.timeout = setTimeout(check, 50)
  }
  CodeMirror.on(window, "mouseup", state.hurry)
  CodeMirror.on(window, "keyup", state.hurry)
}

function stopListening(_cm, state) {
  clearTimeout(state.timeout)
  CodeMirror.off(window, "mouseup", state.hurry)
  CodeMirror.off(window, "keyup", state.hurry)
}

My problem can be resolved in some other way (for example, don't instantiate 2 CodeMirror instance). So we can focus on the possible bug itself.
I'm in hurry so can not providing a minimize reproduction demo for now. If you need one please let me know.

**Version:** codemirror: 5.65.12 Browser: Chrome 120.0.6099.217 (I believe this problem is browser-irrelevent) **Problem:** **Code Analysis:** As is shown in code below, when `cm.display.wrapper.offsetHeight == 0` the autoRefresh addon register listeners of mouseup/keyup event on window, and continiously check the wrapper's offsetHeight. As soon as the offsetHeight check pass, these listeners will be unregistered. **Abnormal Situation:** But under the situation that the offsetHeight of wrapper element is keeps 0 during the whole lifetime of a CodeMirror instance, the listener will never be released, causing the instance be referenced by listener on window, and never be gc'd. **My Case:** In my circumstance, I have a dialog contains 2 CodeMirror instance, one of which is hidden by `display: none`. The dialog and its content is destroyed after closing, and re-create if opened again. So the hidden instance might never be rendered, and the wrapper's offsetHeight keeps 0(I'm using CodeMirror constructor to create instance, BTW). I believe attempts to destroy instance have no use to this problem. For the `stopListening` function can only be called by `check`. ```javascript // addon/display/autorefresh.js CodeMirror.defineOption("autoRefresh", false, function(cm, val) { if (cm.state.autoRefresh) { stopListening(cm, cm.state.autoRefresh) cm.state.autoRefresh = null } if (val && cm.display.wrapper.offsetHeight == 0) startListening(cm, cm.state.autoRefresh = {delay: val.delay || 250}) }) function startListening(cm, state) { function check() { // >>What if the offsetHeight is always 0 during the whole lifetime of CodeMirror instance?<< if (cm.display.wrapper.offsetHeight) { stopListening(cm, state) if (cm.display.lastWrapHeight != cm.display.wrapper.clientHeight) cm.refresh() } else { state.timeout = setTimeout(check, state.delay) } } state.timeout = setTimeout(check, state.delay) state.hurry = function() { clearTimeout(state.timeout) state.timeout = setTimeout(check, 50) } CodeMirror.on(window, "mouseup", state.hurry) CodeMirror.on(window, "keyup", state.hurry) } function stopListening(_cm, state) { clearTimeout(state.timeout) CodeMirror.off(window, "mouseup", state.hurry) CodeMirror.off(window, "keyup", state.hurry) } ``` My problem can be resolved in some other way (for example, don't instantiate 2 CodeMirror instance). So we can focus on the possible bug itself. I'm in hurry so can not providing a minimize reproduction demo for now. If you need one please let me know.

marijnh commented

2024-01-27 10:25:16 +01:00

(Migrated from gitlab.com)

mentioned in issue #106

No labels

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

codemirror/codemirror5#7085

No description provided.

Rows
Columns