From: Maryam Ariyan Date: Fri, 23 Mar 2018 16:29:55 +0000 (-0700) Subject: Fixes StringBuilder unbounded size growth in Clear() when we use a mix of Append... X-Git-Tag: accepted/tizen/unified/20190422.045933~2534 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=1ee83de5d83eec5362b4e335a326bfe816afa3a1;p=platform%2Fupstream%2Fcoreclr.git Fixes StringBuilder unbounded size growth in Clear() when we use a mix of Append and Insert (#16926) * 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 --- diff --git a/src/mscorlib/shared/System.Private.CoreLib.Shared.projitems b/src/mscorlib/shared/System.Private.CoreLib.Shared.projitems index 270d06d..f4f8dd3 100644 --- a/src/mscorlib/shared/System.Private.CoreLib.Shared.projitems +++ b/src/mscorlib/shared/System.Private.CoreLib.Shared.projitems @@ -521,6 +521,7 @@ + diff --git a/src/mscorlib/shared/System/Text/StringBuilder.Debug.cs b/src/mscorlib/shared/System/Text/StringBuilder.Debug.cs new file mode 100644 index 0000000..c2c8c9f --- /dev/null +++ b/src/mscorlib/shared/System/Text/StringBuilder.Debug.cs @@ -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) + '|'); + } + } +} diff --git a/src/mscorlib/shared/System/Text/StringBuilder.cs b/src/mscorlib/shared/System/Text/StringBuilder.cs index 185e5a4..1a32084 100644 --- a/src/mscorlib/shared/System/Text/StringBuilder.cs +++ b/src/mscorlib/shared/System/Text/StringBuilder.cs @@ -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); } }