Skip to content

Commit 47c9de9

Browse files
rmdmjasnell
authored andcommitted
assert: fix deepEqual RangeError: Maximum call stack size exceeded
Fixes: #13314 Refs: #6416 This commit changes semantics of the memos cycles tracker. Before the change it was used to track all currently wisited nodes of an object tree, which is a bit shifted from its original intention of tracking cycles. The change brings intended semantics, by tracking only objects of the current branch of the object tree. PR-URL: #13318 Fixes: #13314 Ref: #6416 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joyee Cheung <[email protected]>
1 parent d1b39d9 commit 47c9de9

File tree

3 files changed

+38
-13
lines changed

3 files changed

+38
-13
lines changed

lib/assert.js

+16-13
Original file line numberDiff line numberDiff line change
@@ -262,24 +262,27 @@ function _deepEqual(actual, expected, strict, memos) {
262262
// Use memos to handle cycles.
263263
if (!memos) {
264264
memos = {
265-
actual: { map: new Map(), position: 0 },
266-
expected: { map: new Map(), position: 0 }
265+
actual: new Map(),
266+
expected: new Map(),
267+
position: 0
267268
};
268-
}
269-
270-
const actualPosition = memos.actual.map.get(actual);
271-
if (actualPosition !== undefined) {
272-
if (actualPosition === memos.expected.map.get(expected)) {
273-
return true;
274-
}
275269
} else {
276-
memos.actual.map.set(actual, memos.actual.position++);
270+
memos.position++;
277271
}
278-
if (!memos.expected.map.has(expected)) {
279-
memos.expected.map.set(expected, memos.expected.position++);
272+
273+
if (memos.actual.has(actual)) {
274+
return memos.actual.get(actual) === memos.expected.get(expected);
280275
}
281276

282-
return objEquiv(actual, expected, strict, memos);
277+
memos.actual.set(actual, memos.position);
278+
memos.expected.set(expected, memos.position);
279+
280+
const areEq = objEquiv(actual, expected, strict, memos);
281+
282+
memos.actual.delete(actual);
283+
memos.expected.delete(expected);
284+
285+
return areEq;
283286
}
284287

285288
function setHasSimilarElement(set, val1, strict, memo) {

test/parallel/test-assert-deep.js

+10
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,16 @@ assertNotDeepOrStrict(new Set([1, 2, 3, 4]), new Set([1, 2, 3]));
158158
assertDeepAndStrictEqual(new Set(['1', '2', '3']), new Set(['1', '2', '3']));
159159
assertDeepAndStrictEqual(new Set([[1, 2], [3, 4]]), new Set([[3, 4], [1, 2]]));
160160

161+
const a = [ 1, 2 ];
162+
const b = [ 3, 4 ];
163+
const c = [ 1, 2 ];
164+
const d = [ 3, 4 ];
165+
166+
assertDeepAndStrictEqual(
167+
{ a: a, b: b, s: new Set([a, b]) },
168+
{ a: c, b: d, s: new Set([d, c]) }
169+
);
170+
161171
assertDeepAndStrictEqual(new Map([[1, 1], [2, 2]]), new Map([[1, 1], [2, 2]]));
162172
assertDeepAndStrictEqual(new Map([[1, 1], [2, 2]]), new Map([[2, 2], [1, 1]]));
163173
assertNotDeepOrStrict(new Map([[1, 1], [2, 2]]), new Map([[1, 2], [2, 1]]));

test/parallel/test-assert.js

+12
Original file line numberDiff line numberDiff line change
@@ -535,6 +535,18 @@ a.throws(makeBlock(thrower, TypeError), function(err) {
535535

536536
a.throws(makeBlock(a.deepEqual, d, e), /AssertionError/);
537537
a.throws(makeBlock(a.deepStrictEqual, d, e), /AssertionError/);
538+
539+
// https://github.com/nodejs/node/issues/13314
540+
const f = {};
541+
f.ref = f;
542+
543+
const g = {};
544+
g.ref = g;
545+
546+
const h = {ref: g};
547+
548+
a.throws(makeBlock(a.deepEqual, f, h), /AssertionError/);
549+
a.throws(makeBlock(a.deepStrictEqual, f, h), /AssertionError/);
538550
}
539551
// GH-7178. Ensure reflexivity of deepEqual with `arguments` objects.
540552
const args = (function() { return arguments; })();

0 commit comments

Comments
 (0)