Skip to content

Commit

Permalink
Fix leaking memory during parsing
Browse files Browse the repository at this point in the history
The monkey-patch for maintaining the open element stack, as modified by commit 75a921e, maintained a reference to the most recent JSDOMParse5Adapter through the push/pop closures. This means there was always a reference from the rooted OpenElementStack.prototype.{push,pop} functions to the most recent JSDOMParse5Adapter and its related objects. Since one of the related objects was an element, which has a pointer to a document and window, essentially the whole JSDOM would be retained.

This commit instead uses the this.treeAdapter pointer, which already exists in parse5. Doing so also allows us to move the monkeypatch back into the top level, instead of establishing it on construction of each tree adapter.

Closes #2831. Closes #2825. Helps with the root cause of jestjs/jest#9507.
  • Loading branch information
domenic authored Feb 16, 2020
1 parent d18e339 commit c3227ce
Showing 1 changed file with 26 additions and 25 deletions.
51 changes: 26 additions & 25 deletions lib/jsdom/browser/parser/html.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,40 +12,41 @@ const nodeTypes = require("../../living/node-type");

const serializationAdapter = require("../../living/domparsing/parse5-adapter-serialization");

// Horrible monkey-patch to implement https://github.com/inikulin/parse5/issues/237 and
// https://github.com/inikulin/parse5/issues/285.
const OpenElementStack = require("parse5/lib/parser/open-element-stack");
const OpenElementStackOriginalPop = OpenElementStack.prototype.pop;
const OpenElementStackOriginalPush = OpenElementStack.prototype.push;

const openElementStackOriginalPush = OpenElementStack.prototype.push;
OpenElementStack.prototype.push = function (...args) {
openElementStackOriginalPush.apply(this, args);
this.treeAdapter._currentElement = this.current;

const after = this.items[this.stackTop];
if (after._pushedOnStackOfOpenElements) {
after._pushedOnStackOfOpenElements();
}
};

const openElementStackOriginalPop = OpenElementStack.prototype.pop;
OpenElementStack.prototype.pop = function (...args) {
const before = this.items[this.stackTop];

openElementStackOriginalPop.apply(this, args);
this.treeAdapter._currentElement = this.current;

if (before._poppedOffStackOfOpenElements) {
before._poppedOffStackOfOpenElements();
}
};

class JSDOMParse5Adapter {
constructor(documentImpl) {
this._documentImpl = documentImpl;
this._globalObject = documentImpl._globalObject;

// Since the createElement hook doesn't provide the parent element, we keep track of this using _currentElement:
// https://github.com/inikulin/parse5/issues/285
// https://github.com/inikulin/parse5/issues/285. See above horrible monkey-patch for how this is maintained.
this._currentElement = undefined;

// Horrible monkey-patch to implement https://github.com/inikulin/parse5/issues/237
const adapter = this;
OpenElementStack.prototype.push = function (...args) {
OpenElementStackOriginalPush.apply(this, args);
adapter._currentElement = this.current;

const after = this.items[this.stackTop];
if (after._pushedOnStackOfOpenElements) {
after._pushedOnStackOfOpenElements();
}
};
OpenElementStack.prototype.pop = function (...args) {
const before = this.items[this.stackTop];

OpenElementStackOriginalPop.apply(this, args);
adapter._currentElement = this.current;

if (before._poppedOffStackOfOpenElements) {
before._poppedOffStackOfOpenElements();
}
};
}

_ownerDocument() {
Expand Down

0 comments on commit c3227ce

Please sign in to comment.