fix ReactiveProperty more

Cysharp · Mar 10, 2024 · 0f125f4 · 0f125f4
1 parent 942f111
commit 0f125f4
Show file tree

Hide file tree

Showing 3 changed files with 139 additions and 17 deletions.
diff --git a/sandbox/ConsoleApp1/Program.cs b/sandbox/ConsoleApp1/Program.cs
@@ -12,19 +12,61 @@
 using System.Threading.Channels;
 
 
-
-var p1 = new ReactiveProperty<int>();
-p1.Skip(1).Subscribe(x => Debug.Log("[P1]" + x));
-
-var d = p1.Skip(1).Subscribe(x => Debug.Log("[P2]" + x));
-
-d.Dispose();
-
-p1.Skip(1).Subscribe(x => Debug.Log("[P3]" + x));
-
-p1.Value = 1;
-p1.Value = 2;
-
+var count = 0;
+var r = new ReactiveProperty<int>(count);
+var ctsA = new CancellationTokenSource();
+var ctsB = new CancellationTokenSource();
+var ctsC = new CancellationTokenSource();
+r.Subscribe(x => Debug.Log($"A = {x}")).RegisterTo(ctsA.Token);
+r.Subscribe(x => Debug.Log($"B = {x}")).RegisterTo(ctsB.Token);
+r.Subscribe(x => Debug.Log($"C = {x}")).RegisterTo(ctsC.Token);
+ctsA.Cancel();
+ctsA.Dispose();
+Debug.Log("A disposed");
+
+ctsA = new CancellationTokenSource();
+r.Subscribe(x => Debug.Log($"A = {x}")).RegisterTo(ctsA.Token);
+Debug.Log("A re-registered");
+ctsA.Cancel();
+ctsA.Dispose();
+Debug.Log("A disposed");
+ctsA = new CancellationTokenSource();
+r.Subscribe(x => Debug.Log($"A = {x}")).RegisterTo(ctsA.Token);
+Debug.Log("A re-registered");
+r.Value = ++count;
+r.Value = ++count;
+
+// Expected operation:
+// A = 0
+// B = 0
+// C = 0
+// A disposed
+// A = 0
+// A re-registered
+// A disposed
+// A = 0
+// A re-registered
+// A = 1
+// B = 1
+// C = 1
+// A = 2
+// B = 2
+// C = 2
+
+// However, the actual operation is:
+// A = 0
+// B = 0
+// C = 0
+// A disposed
+// A = 0
+// A re-registered
+// A disposed
+// A = 0
+// A re-registered
+// B = 1
+// C = 1
+// B = 2
+// C = 2
 
 public static class Debug
 {

diff --git a/src/R3/ReactiveProperty.cs b/src/R3/ReactiveProperty.cs
@@ -237,6 +237,21 @@ public int NodeCount
         }
     }
 
+    ObserverNode[] Nodes
+    {
+        get
+        {
+            var list = new List<ObserverNode>();
+            var node = root;
+            while (node != null)
+            {
+                list.Add(node);
+                node = node.Next;
+            }
+            return list.ToArray();
+        }
+    }
+
 #endif
 
     sealed class ObserverNode : IDisposable
@@ -249,11 +264,18 @@ sealed class ObserverNode : IDisposable
         public ObserverNode? Next { get; set; }
 
 
-        // for debugging property
+        // for debugging field/property
 #if DEBUG
+        static int idGenerator;
+        public int Id = Interlocked.Increment(ref idGenerator);
         public bool IsRootNode => parent?.root == this;
         public bool IsSingleRootNode => IsRootNode && Previous == this;
         public bool HasNext => Previous == this;
+
+        public override string ToString()
+        {
+            return $"{Previous.Id} -> ({Id}) -> {Next?.Id}";
+        }
 #endif
 
         public ObserverNode(ReactiveProperty<T> parent, Observer<T> observer)
@@ -317,10 +339,18 @@ public void Dispose()
                     }
                 }
 
-                // root is single node
-                if (p.root != null && p.root.Next == null)
+                // modify root
+                if (p.root != null)
                 {
-                    p.root.Previous = p.root;
+                    // root is single node
+                    if (p.root.Next == null)
+                    {
+                        p.root.Previous = p.root;
+                    }
+                    else if (p.root.Previous == this)
+                    {
+                        p.root.Previous = prev;
+                    }
                 }
             }
         }

diff --git a/tests/R3.Tests/ReactivePropertyTest.cs b/tests/R3.Tests/ReactivePropertyTest.cs
@@ -276,4 +276,54 @@ public void RootChangedFromSecond()
         p1List.AssertEqual([1, 2]);
         p3List.AssertEqual([1, 2]);
     }
+
+    [Fact]
+    public void RemoveLastNode()
+    {
+        var log = new List<string>();
+
+        var count = 0;
+        var r = new ReactiveProperty<int>(count);
+        var ctsA = new CancellationTokenSource();
+        var ctsB = new CancellationTokenSource();
+        var ctsC = new CancellationTokenSource();
+        r.Subscribe(x => log.Add($"A = {x}")).RegisterTo(ctsA.Token);
+        r.Subscribe(x => log.Add($"B = {x}")).RegisterTo(ctsB.Token);
+        r.Subscribe(x => log.Add($"C = {x}")).RegisterTo(ctsC.Token);
+        ctsA.Cancel();
+        ctsA.Dispose();
+        log.Add("A disposed");
+
+        ctsA = new CancellationTokenSource();
+        r.Subscribe(x => log.Add($"A = {x}")).RegisterTo(ctsA.Token);
+        log.Add("A re-registered");
+        ctsA.Cancel();
+        ctsA.Dispose();
+        log.Add("A disposed");
+        ctsA = new CancellationTokenSource();
+        r.Subscribe(x => log.Add($"A = {x}")).RegisterTo(ctsA.Token);
+        log.Add("A re-registered");
+        r.Value = ++count;
+        r.Value = ++count;
+
+        var actual = string.Join(Environment.NewLine, log);
+
+        actual.Should().Be("""
+A = 0
+B = 0
+C = 0
+A disposed
+A = 0
+A re-registered
+A disposed
+A = 0
+A re-registered
+B = 1
+C = 1
+A = 1
+B = 2
+C = 2
+A = 2
+""");
+    }
 }