Skip to content

Commit 8db9d43

Browse files
authored
assert,util: fix stale nested cycle memo entries
Temporary nested cycle-tracking entries could remain in the memory set after a successful comparison. If a later sibling comparison reused one of those objects, deepStrictEqual could incorrectly fail for equivalent structures. This cleans up the temporary nested entries after the nested comparison returns. Fixes: #62422 PR-URL: #62509 Fixes: #62422 Reviewed-By: Jordan Harband <ljharb@gmail.com>
1 parent b52102f commit 8db9d43

File tree

2 files changed

+42
-0
lines changed

2 files changed

+42
-0
lines changed

lib/internal/util/comparisons.js

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -529,6 +529,10 @@ function handleCycles(val1, val2, mode, keys1, keys2, memos, iterationType) {
529529
memos.deep = true;
530530
const result = objEquiv(val1, val2, mode, keys1, keys2, memos, iterationType);
531531
memos.deep = false;
532+
if (memos.set !== undefined) {
533+
memos.set.delete(memos.c);
534+
memos.set.delete(memos.d);
535+
}
532536
return result;
533537
}
534538
memos.set = new SafeSet();

test/parallel/test-assert-deep.js

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,14 @@ function assertOnlyDeepEqual(a, b, err) {
260260
assert.throws(partial, err || { code: 'ERR_ASSERTION' });
261261
}
262262

263+
function activateMemoizedCycleDetection() {
264+
const circA = {};
265+
circA.self = circA;
266+
const circB = {};
267+
circB.self = circB;
268+
assert.deepStrictEqual(circA, circB);
269+
}
270+
263271
test('es6 Maps and Sets', () => {
264272
assertDeepAndStrictEqual(new Set(), new Set());
265273
assertDeepAndStrictEqual(new Map(), new Map());
@@ -609,6 +617,36 @@ test('GH-14441. Circular structures should be consistent', () => {
609617
}
610618
});
611619

620+
test('deepStrictEqual handles shared expected array elements after cycle detection', () => {
621+
const sharedExpected = { outer: { inner: 0 } };
622+
const actualValues = [{ outer: { inner: 0 } }, { outer: { inner: 0 } }];
623+
const expectedValues = [sharedExpected, sharedExpected];
624+
625+
activateMemoizedCycleDetection();
626+
627+
assertDeepAndStrictEqual(actualValues[0], expectedValues[0]);
628+
assertDeepAndStrictEqual(actualValues[1], expectedValues[1]);
629+
assertDeepAndStrictEqual(actualValues, expectedValues);
630+
});
631+
632+
test('deepStrictEqual handles cross-root aliases after cycle detection', () => {
633+
activateMemoizedCycleDetection();
634+
635+
const nestedExpected = {};
636+
nestedExpected.loop = nestedExpected;
637+
nestedExpected.payload = { value: 1 };
638+
639+
const expected = {};
640+
expected.loop = nestedExpected;
641+
expected.payload = { value: 1 };
642+
643+
const actual = {};
644+
actual.loop = expected;
645+
actual.payload = { value: 1 };
646+
647+
assertDeepAndStrictEqual(actual, expected);
648+
});
649+
612650
// https://github.com/nodejs/node-v0.x-archive/pull/7178
613651
test('Ensure reflexivity of deepEqual with `arguments` objects.', () => {
614652
const args = (function() { return arguments; })();

0 commit comments

Comments
 (0)