Fixes StringBuilder unbounded size growth in Clear() when we use a mix of Append...
authorMaryam Ariyan <maryam.ariyan@microsoft.com>
Fri, 23 Mar 2018 16:29:55 +0000 (09:29 -0700)
committerDan Moseley <danmose@microsoft.com>
Fri, 23 Mar 2018 16:29:55 +0000 (09:29 -0700)
* Fixing Clear infinitely growing when we combine usage of Insert and Append. Fixes #27625

* Move debug code to StringBuilder.Debug.cs and Applied code review feedback

* Adding missing debug condition check

* Adding comments and moving Condition on projitems after filename

* Moving the infinite capacity growth fix to Length setter

* Removing originalCapacity and Debug.Assert

* Applying PR feedbacks

* Minor cleanup

* simplifying to single loop

* keeping just one method for ShowChunks function

src/mscorlib/shared/System.Private.CoreLib.Shared.projitems
src/mscorlib/shared/System/Text/StringBuilder.Debug.cs [new file with mode: 0644]
src/mscorlib/shared/System/Text/StringBuilder.cs

index 270d06d..f4f8dd3 100644 (file)
     <Compile Include="$(MSBuildThisFileDirectory)System\Text\Latin1Encoding.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Text\NormalizationForm.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Text\StringBuilder.cs" />
+    <Compile Include="$(MSBuildThisFileDirectory)System\Text\StringBuilder.Debug.cs" Condition="'$(Configuration)' == 'Debug'" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Text\UnicodeEncoding.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Text\UTF32Encoding.cs" />
     <Compile Include="$(MSBuildThisFileDirectory)System\Text\UTF7Encoding.cs" />
diff --git a/src/mscorlib/shared/System/Text/StringBuilder.Debug.cs b/src/mscorlib/shared/System/Text/StringBuilder.Debug.cs
new file mode 100644 (file)
index 0000000..c2c8c9f
--- /dev/null
@@ -0,0 +1,39 @@
+// Licensed to the .NET Foundation under one or more agreements.
+// The .NET Foundation licenses this file to you under the MIT license.
+// See the LICENSE file in the project root for more information.
+
+using System.Collections.Generic;
+using System.Diagnostics;
+using System.Runtime.Serialization;
+
+namespace System.Text
+{
+    public sealed partial class StringBuilder
+    {
+        private void ShowChunks(int maxChunksToShow = 10)
+        {
+            int count = 0;
+            StringBuilder head = this, current = this;
+            while (current != null)
+            {
+                if (count < maxChunksToShow)
+                {
+                    count++;
+                }
+                else
+                {
+                    head = head.m_ChunkPrevious;
+                }
+                current = current.m_ChunkPrevious;
+            }
+            current = head;
+            string[] chunks = new string[count];
+            for (int i = count; i > 0; i--)
+            {
+                chunks[i - 1] = new string(current.m_ChunkChars).Replace('\0', '.');
+                current = current.m_ChunkPrevious;
+            }
+            Debug.WriteLine('|' + string.Join('|', chunks) + '|');
+        }
+    }
+}
index 185e5a4..1a32084 100644 (file)
@@ -462,13 +462,10 @@ namespace System.Text
                     throw new ArgumentOutOfRangeException(nameof(value), SR.ArgumentOutOfRange_SmallCapacity);
                 }
 
-                int originalCapacity = Capacity;
-
                 if (value == 0 && m_ChunkPrevious == null)
                 {
                     m_ChunkLength = 0;
                     m_ChunkOffset = 0;
-                    Debug.Assert(Capacity >= originalCapacity);
                     return;
                 }
 
@@ -485,7 +482,10 @@ namespace System.Text
                     {
                         // We crossed a chunk boundary when reducing the Length. We must replace this middle-chunk with a new larger chunk,
                         // to ensure the original capacity is preserved.
-                        int newLen = originalCapacity - chunk.m_ChunkOffset;
+
+                        // Avoid possible infinite capacity growth.  See https://github.com/dotnet/coreclr/pull/16926
+                        int capacityToPreserve = Math.Min(Capacity, Math.Max(Length * 6 / 5, m_ChunkChars.Length));
+                        int newLen = capacityToPreserve - chunk.m_ChunkOffset;
                         char[] newArray = new char[newLen];
 
                         Debug.Assert(newLen > chunk.m_ChunkChars.Length, "The new chunk should be larger than the one it is replacing.");
@@ -498,7 +498,6 @@ namespace System.Text
                     m_ChunkLength = value - chunk.m_ChunkOffset;
                     AssertInvariants();
                 }
-                Debug.Assert(Capacity >= originalCapacity);
             }
         }