From 3881703e01b70942d0915a51ff2bf70077761aa3 Mon Sep 17 00:00:00 2001 From: Igor Sadikov Date: Fri, 18 Jan 2019 17:58:09 -0500 Subject: [PATCH] Fix xterm.js resource leaks in split pane (#3409) --- lib/components/term.js | 33 ++++++++++++++++----------------- lib/components/terms.js | 11 +++++++++-- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/lib/components/term.js b/lib/components/term.js index e678a4f2..978f5eb7 100644 --- a/lib/components/term.js +++ b/lib/components/term.js @@ -76,13 +76,11 @@ export default class Term extends React.PureComponent { constructor(props) { super(props); props.ref_(props.uid, this); - this.termRef = null; this.termWrapperRef = null; this.termRect = null; this.onOpen = this.onOpen.bind(this); this.onWindowResize = this.onWindowResize.bind(this); this.onWindowPaste = this.onWindowPaste.bind(this); - this.onTermRef = this.onTermRef.bind(this); this.onTermWrapperRef = this.onTermWrapperRef.bind(this); this.onMouseUp = this.onMouseUp.bind(this); this.termOptions = {}; @@ -94,15 +92,21 @@ export default class Term extends React.PureComponent { this.termOptions = getTermOptions(props); this.term = props.term || new Terminal(this.termOptions); - this.term.attachCustomKeyEventHandler(this.keyboardHandler); - this.term.open(this.termRef); - this.term.webLinksInit(); - this.term.winptyCompatInit(); - if (props.term) { - //We need to set options again after reattaching an existing term - Object.keys(this.termOptions).forEach(option => this.term.setOption(option, this.termOptions[option])); + // The parent element for the terminal is attached and removed manually so + // that we can preserve it across mounts and unmounts of the component + let parent = props.term ? props.term._core._parent : document.createElement('div'); + parent.className = 'term_fit term_term'; + + this.termWrapperRef.appendChild(parent); + + if (!props.term) { + this.term.attachCustomKeyEventHandler(this.keyboardHandler); + this.term.open(parent); + this.term.webLinksInit(); + this.term.winptyCompatInit(); } + if (this.props.isTermActive) { this.term.focus(); } @@ -302,12 +306,9 @@ export default class Term extends React.PureComponent { this.termWrapperRef = component; } - onTermRef(component) { - this.termRef = component; - } - componentWillUnmount() { terms[this.props.uid] = null; + this.termWrapperRef.removeChild(this.term._core._parent); this.props.ref_(this.props.uid, null); // to clean up the terminal, we remove the listeners @@ -334,12 +335,10 @@ export default class Term extends React.PureComponent { onMouseUp={this.onMouseUp} > {this.props.customChildrenBefore} -
-
-
+
{this.props.customChildren} -