From 6e95c19b3e8a16f5431870a2fdaef61c25a5c7ba Mon Sep 17 00:00:00 2001
From: Bartosz Sypytkowski <b.sypytkowski@gmail.com>
Date: Thu, 15 Jun 2023 09:50:06 +0200
Subject: [PATCH] prevent infinite loops when deepObserve is called over links
 with circular references

---
 src/structs/ContentType.js  | 15 ++++++++++++++-
 src/types/AbstractType.js   | 18 ++++++++++--------
 src/types/YWeakLink.js      | 13 -------------
 tests/y-weak-links.tests.js | 21 +++++++++++++++------
 4 files changed, 39 insertions(+), 28 deletions(-)

diff --git a/src/structs/ContentType.js b/src/structs/ContentType.js
index 479ead2c..42ada06e 100644
--- a/src/structs/ContentType.js
+++ b/src/structs/ContentType.js
@@ -1,4 +1,5 @@
 
+import { WeakLink } from 'yjs'
 import {
   readYArray,
   readYMap,
@@ -107,7 +108,19 @@ export class ContentType {
    * @param {Transaction} transaction
    */
   delete (transaction) {
-    this.type._delete(transaction) // call custom destructor on AbstractType
+    if (this.type.constructor === WeakLink) {
+      // when removing weak links, remove references to them 
+      // from type they're pointing to
+      const type = /** @type {WeakLink<any>} */ (this.type);
+      if (type._linkedItem !== null && !type._linkedItem.deleted) {
+        const item = /** @type {Item} */ (type._linkedItem)
+        if (item.linkedBy !== null) {
+          item.linkedBy.delete(type)
+        }
+        type._linkedItem = null
+      }
+    }
+
     let item = this.type._start
     while (item !== null) {
       if (!item.deleted) {
diff --git a/src/types/AbstractType.js b/src/types/AbstractType.js
index fd30944e..0dbe0cc7 100644
--- a/src/types/AbstractType.js
+++ b/src/types/AbstractType.js
@@ -233,8 +233,9 @@ export const getTypeChildren = t => {
  * @param {AbstractType<EventType>} type
  * @param {Transaction} transaction
  * @param {EventType} event
+ * @param {Set<YWeakLink<any>>|null} visitedLinks
  */
-export const callTypeObservers = (type, transaction, event) => {
+export const callTypeObservers = (type, transaction, event, visitedLinks = null) => {
   const changedType = type
   const changedParentTypes = transaction.changedParentTypes
   while (true) {
@@ -243,9 +244,15 @@ export const callTypeObservers = (type, transaction, event) => {
     if (type._item === null) {
       break
     } else if (type._item.linkedBy !== null) {
+      if (visitedLinks === null) {
+        visitedLinks = new Set()
+      }
       for (let link of type._item.linkedBy) {
-        // recursive call
-        callTypeObservers(link, transaction, /** @type {any} */ (event))
+        if (!visitedLinks.has(link)) {
+          visitedLinks.add(link)
+          // recursive call
+          callTypeObservers(link, transaction, /** @type {any} */ (event), visitedLinks)
+        }
       }
     }
     type = /** @type {AbstractType<any>} */ (type._item.parent)
@@ -314,11 +321,6 @@ export class AbstractType {
     this._item = item
   }
 
-  /**
-   * @param {Transaction} transaction 
-   */
-  _delete (transaction) { }
-
   /**
    * @return {AbstractType<EventType>}
    */
diff --git a/src/types/YWeakLink.js b/src/types/YWeakLink.js
index 76b319f8..5cbf106d 100644
--- a/src/types/YWeakLink.js
+++ b/src/types/YWeakLink.js
@@ -98,19 +98,6 @@ export class YWeakLink extends AbstractType {
       })
     }
   }
-  
-  /**
-   * @param {Transaction} transaction
-   */
-  _delete (transaction) {
-    if (this._item !== null && this._linkedItem !== null && !this._linkedItem.deleted) {
-      const item = /** @type {Item} */ (this._linkedItem)
-      if (item.linkedBy !== null) {
-        item.linkedBy.delete(this)
-      }
-      this._linkedItem = null
-    }
-  }
 
   /**
    * @return {YWeakLink<T>}
diff --git a/tests/y-weak-links.tests.js b/tests/y-weak-links.tests.js
index 3b0aeb49..ef32d405 100644
--- a/tests/y-weak-links.tests.js
+++ b/tests/y-weak-links.tests.js
@@ -347,7 +347,7 @@ export const testDeepObserveArray = tc => {
 /**
  * @param {t.TestCase} tc
  */
- const testDeepObserveRecursive = tc => {
+export const testDeepObserveRecursive = tc => {
   // test observers in a face of linked chains of values
   const doc = new Y.Doc()
   const root = doc.getArray('array')
@@ -365,9 +365,9 @@ export const testDeepObserveArray = tc => {
   const l2 = root.link(2)
 
   // create cyclic reference between links
-  m0.set('k1', m1)
-  m1.set('k2', m2)
-  m2.set('k0', m0)
+  m0.set('k1', l1)
+  m1.set('k2', l2)
+  m2.set('k0', l0)
 
   /**
    * @type {Array<any>}
@@ -376,9 +376,18 @@ export const testDeepObserveArray = tc => {
   m0.observeDeep((e) => events = e)
 
   m1.set('test-key1', 'value1')
-  t.compare(events, [{}]) //TODO
+  t.compare(events.length, 1)
+  t.compare(events[0].target, m1)
+  t.compare(events[0].keys, new Map([['test-key1', {action:'add', oldValue: undefined}]]))
   
   events = []
   m2.set('test-key2', 'value2')
-  t.compare(events, [{}]) //TODO
+  t.compare(events.length, 1)
+  t.compare(events[0].target, m2)
+  t.compare(events[0].keys, new Map([['test-key2', {action:'add', oldValue: undefined}]]))
+
+  m1.delete('test-key1')
+  t.compare(events.length, 1)
+  t.compare(events[0].target, m1)
+  t.compare(events[0].keys, new Map([['test-key1', {action:'delete', oldValue: undefined}]]))
 }
\ No newline at end of file