Ensure that CallCollection doesn't leak memory for deleted entries

nsubstitute · Nov 27, 2016 · 77e8c15 · 77e8c15
1 parent 39aedac
commit 77e8c15
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 4 deletions.
diff --git a/Source/NSubstitute.Acceptance.Specs/PerfTests.cs b/Source/NSubstitute.Acceptance.Specs/PerfTests.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Diagnostics;
+using System.Threading;
 using NUnit.Framework;
 
 namespace NSubstitute.Acceptance.Specs
@@ -54,7 +55,24 @@ public void TimeBasicOperationsWithGenerics()
             Console.WriteLine("{0}", watch.ElapsedMilliseconds);
         }
 
+        [Test]
+        [Ignore]
+        public void Muiltiple_return_configurations_dont_lead_to_memory_leak()
+        {
+            const int bufferSize = 100000000; //100 MB
+
+            var subs = Substitute.For<IByteArraySource>();
+
+            //1000 chunks each 100 MB will require 100GB. If leak is present - OOM should be thrown.
+            for (int i = 0; i < 1000; i++)
+            {
+                subs.GetArray().Returns(new byte[bufferSize]);
+                Thread.Sleep(500);
+            }
+        }
+
         public interface IFoo { int GetInt(string s); }
         public interface IBar { int GetInt<T>(T t); }
+        public interface IByteArraySource { byte[] GetArray(); }
     }
 }
diff --git a/Source/NSubstitute/Core/CallCollection.cs b/Source/NSubstitute/Core/CallCollection.cs
@@ -19,10 +19,10 @@ public void Delete(ICall call)
         {
             if (call == null) throw new ArgumentNullException(nameof(call));
 
-            var callWrapper = _callWrappers.FirstOrDefault(w => w.Call.Equals(call));
+            var callWrapper = _callWrappers.FirstOrDefault(w => !w.IsDeleted && call.Equals(w.Call));
             if (callWrapper == null) throw new InvalidOperationException("Collection doesn't contain the call.");
 
-            callWrapper.IsDeleted = true;
+            callWrapper.Delete();
         }
 
         public IEnumerable<ICall> AllCalls()
@@ -41,16 +41,22 @@ public void Clear()
         /// <summary>
         /// Wrapper to track that particular entry was deleted.
         /// That is needed because concurrent collections don't have a Delete method.
+        /// We null the hold value to not leak memory.
         /// </summary>
         private class CallWrapper
         {
-            public ICall Call { get; }
-            public bool IsDeleted { get; set; }
+            public ICall Call { get; private set; }
+            public bool IsDeleted => Call == null;
 
             public CallWrapper(ICall call)
             {
                 Call = call;
             }
+
+            public void Delete()
+            {
+                Call = null;
+            }
         }
     }
 }