fixed: Bug where array pool is not returned and could even be overwritten

Author: linkdotnet

src/LinkDotNet.StringBuilder/ValueStringBuilder.Replace.cs

@@ -90,7 +90,7 @@ public void ReplaceGeneric<T>(ReadOnlySpan<char> oldValue, T newValue, int start
             var tempBuffer = ArrayPool<char>.Shared.Rent(24);
             if (spanFormattable.TryFormat(tempBuffer, out var written, default, null))
             {
-                Replace(oldValue, tempBuffer.AsSpan()[..written], startIndex, count);
+                Replace(oldValue, tempBuffer.AsSpan(0, written), startIndex, count);
             }
 
             ArrayPool<char>.Shared.Return(tempBuffer);

src/LinkDotNet.StringBuilder/ValueStringBuilder.cs

@@ -15,6 +15,7 @@ public ref partial struct ValueStringBuilder
 {
     private int bufferPosition;
     private Span<char> buffer;
+    private char[]? arrayFromPool;
 
     /// <summary>
     /// Initializes a new instance of the <see cref="ValueStringBuilder"/> struct.
@@ -23,7 +24,9 @@ public ref partial struct ValueStringBuilder
     public ValueStringBuilder()
     {
         bufferPosition = 0;
-        buffer = new char[32];
+        buffer = default;
+        arrayFromPool = null;
+        Grow(32);
     }
 
     /// <summary>
@@ -35,6 +38,7 @@ public ValueStringBuilder(Span<char> initialBuffer)
     {
         bufferPosition = 0;
         buffer = initialBuffer;
+        arrayFromPool = null;
     }
 
     /// <summary>
@@ -79,7 +83,7 @@ public ValueStringBuilder(Span<char> initialBuffer)
     /// <remarks>
     /// This method is used for use-cases where the user wants to use "fixed" calls like the following:
     /// <code>
-    /// var stringBuilder = new ValueStringBuilder();
+    /// using var stringBuilder = new ValueStringBuilder();
     /// stringBuilder.Append("Hello World");
     /// fixed (var* buffer = stringBuilder) { ... }
     /// </code>
@@ -238,6 +242,18 @@ public int LastIndexOf(ReadOnlySpan<char> word, int startIndex)
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     public bool Contains(ReadOnlySpan<char> word) => IndexOf(word) != -1;
 
+    /// <summary>
+    /// Disposes the instance and returns rented buffer from an array pool if needed.
+    /// </summary>
+    public void Dispose()
+    {
+        var toReturn = arrayFromPool;
+        if (toReturn != null)
+        {
+            ArrayPool<char>.Shared.Return(toReturn);
+        }
+    }
+
     [MethodImpl(MethodImplOptions.AggressiveInlining)]
     private void Grow(int capacity = 0)
     {
@@ -248,7 +264,12 @@ private void Grow(int capacity = 0)
         var newSize = capacity > 0 ? capacity : currentSize * 2;
         var rented = ArrayPool<char>.Shared.Rent(newSize);
         buffer.CopyTo(rented);
-        buffer = rented;
-        ArrayPool<char>.Shared.Return(rented);
+        var toReturn = arrayFromPool;
+        buffer = arrayFromPool = rented;
+
+        if (toReturn != null)
+        {
+            ArrayPool<char>.Shared.Return(toReturn);
+        }
     }
 }

tests/LinkDotNet.StringBuilder.Benchmarks/AppendBenchmark.cs

@@ -20,7 +20,7 @@ public string DotNetStringBuilder()
     [Benchmark]
     public string ValueStringBuilder()
     {
-        var builder = new ValueStringBuilder();
+        using var builder = new ValueStringBuilder();
         builder.AppendLine("That is the first line of our benchmark.");
         builder.AppendLine("We can multiple stuff in here if want.");
         builder.AppendLine("We can multiple stuff in here if want.");

tests/LinkDotNet.StringBuilder.Benchmarks/AppendValueTypesBenchmark.cs

@@ -27,7 +27,7 @@ public string DotNetStringBuilder()
     [Benchmark]
     public string ValueStringBuilder()
     {
-        var builder = new ValueStringBuilder();
+        using var builder = new ValueStringBuilder();
 
         for (var i = 0; i < 25; i++)
         {

tests/LinkDotNet.StringBuilder.Benchmarks/ReplaceBenchmark.cs

@@ -55,7 +55,7 @@ public string DotNetStringBuilder()
     [Benchmark]
     public string ValueStringBuilder()
     {
-        var builder = new ValueStringBuilder();
+        using var builder = new ValueStringBuilder();
         builder.Append(Text);
         builder.Replace("arcu", "some long word");
         builder.Replace("some long word", "arcu");

tests/LinkDotNet.StringBuilder.UnitTests/LinkDotNet.StringBuilder.UnitTests.csproj

@@ -8,14 +8,14 @@
     </PropertyGroup>
 
     <ItemGroup>
-        <PackageReference Include="FluentAssertions" Version="6.6.0" />
+        <PackageReference Include="FluentAssertions" Version="6.7.0" />
         <PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.11.0" />
         <PackageReference Include="xunit" Version="2.4.1" />
-        <PackageReference Include="xunit.runner.visualstudio" Version="2.4.3">
+        <PackageReference Include="xunit.runner.visualstudio" Version="2.4.5">
             <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
             <PrivateAssets>all</PrivateAssets>
         </PackageReference>
-        <PackageReference Include="coverlet.collector" Version="3.1.0">
+        <PackageReference Include="coverlet.collector" Version="3.1.2">
             <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
             <PrivateAssets>all</PrivateAssets>
         </PackageReference>

tests/LinkDotNet.StringBuilder.UnitTests/ValueStringBuilder.Append.Tests.cs

@@ -7,7 +7,7 @@ public class ValueStringBuilderAppendTests
     [Fact]
     public void ShouldAddString()
     {
-        var stringBuilder = new ValueStringBuilder();
+        using var stringBuilder = new ValueStringBuilder();
 
         stringBuilder.Append("That is a string");
 
@@ -17,7 +17,7 @@ public void ShouldAddString()
     [Fact]
     public void ShouldAddMultipleStrings()
     {
-        var stringBuilder = new ValueStringBuilder();
+        using var stringBuilder = new ValueStringBuilder();
 
         stringBuilder.Append("This");
         stringBuilder.Append("is");
@@ -30,7 +30,7 @@ public void ShouldAddMultipleStrings()
     [Fact]
     public void ShouldAddLargeStrings()
     {
-        var stringBuilder = new ValueStringBuilder();
+        using var stringBuilder = new ValueStringBuilder();
 
         stringBuilder.Append(new string('c', 99));
 
@@ -40,7 +40,7 @@ public void ShouldAddLargeStrings()
     [Fact]
     public void ShouldAppendLine()
     {
-        var stringBuilder = new ValueStringBuilder();
+        using var stringBuilder = new ValueStringBuilder();
 
         stringBuilder.AppendLine("Hello");
 
@@ -50,7 +50,7 @@ public void ShouldAppendLine()
     [Fact]
     public void ShouldOnlyAddNewline()
     {
-        var stringBuilder = new ValueStringBuilder();
+        using var stringBuilder = new ValueStringBuilder();
 
         stringBuilder.AppendLine();
 
@@ -60,7 +60,7 @@ public void ShouldOnlyAddNewline()
     [Fact]
     public void ShouldGetIndexIfGiven()
     {
-        var stringBuilder = new ValueStringBuilder();
+        using var stringBuilder = new ValueStringBuilder();
 
         stringBuilder.Append("Hello");
 
@@ -70,7 +70,7 @@ public void ShouldGetIndexIfGiven()
     [Fact]
     public void ShouldAppendFloat()
     {
-        var builder = new ValueStringBuilder();
+        using var builder = new ValueStringBuilder();
 
         builder.Append(2.2f);
 
@@ -80,7 +80,7 @@ public void ShouldAppendFloat()
     [Fact]
     public void ShouldAppendDouble()
     {
-        var builder = new ValueStringBuilder();
+        using var builder = new ValueStringBuilder();
 
         builder.Append(2.2d);
 
@@ -90,7 +90,7 @@ public void ShouldAppendDouble()
     [Fact]
     public void ShouldAppendDecimal()
     {
-        var builder = new ValueStringBuilder();
+        using var builder = new ValueStringBuilder();
 
         builder.Append(2.2m);
 
@@ -100,7 +100,7 @@ public void ShouldAppendDecimal()
     [Fact]
     public void ShouldAppendInteger()
     {
-        var builder = new ValueStringBuilder();
+        using var builder = new ValueStringBuilder();
 
         builder.Append(-2);
 
@@ -110,7 +110,7 @@ public void ShouldAppendInteger()
     [Fact]
     public void ShouldAppendUnsignedInteger()
     {
-        var builder = new ValueStringBuilder();
+        using var builder = new ValueStringBuilder();
 
         builder.Append(2U);
 
@@ -120,7 +120,7 @@ public void ShouldAppendUnsignedInteger()
     [Fact]
     public void ShouldAppendLong()
     {
-        var builder = new ValueStringBuilder();
+        using var builder = new ValueStringBuilder();
 
         builder.Append(2L);
 
@@ -130,7 +130,7 @@ public void ShouldAppendLong()
     [Fact]
     public void ShouldAppendChar()
     {
-        var builder = new ValueStringBuilder();
+        using var builder = new ValueStringBuilder();
 
         builder.Append('2');
 
@@ -140,7 +140,7 @@ public void ShouldAppendChar()
     [Fact]
     public void ShouldAppendShort()
     {
-        var builder = new ValueStringBuilder();
+        using var builder = new ValueStringBuilder();
 
         builder.Append((short)2);
 
@@ -150,7 +150,7 @@ public void ShouldAppendShort()
     [Fact]
     public void ShouldAppendBool()
     {
-        var builder = new ValueStringBuilder();
+        using var builder = new ValueStringBuilder();
 
         builder.Append(true);
 
@@ -160,7 +160,7 @@ public void ShouldAppendBool()
     [Fact]
     public void ShouldAppendByte()
     {
-        var builder = new ValueStringBuilder();
+        using var builder = new ValueStringBuilder();
 
         builder.Append((byte)2);
 
@@ -170,7 +170,7 @@ public void ShouldAppendByte()
     [Fact]
     public void ShouldAppendSignedByte()
     {
-        var builder = new ValueStringBuilder();
+        using var builder = new ValueStringBuilder();
 
         builder.Append((sbyte)2);
 
@@ -180,7 +180,7 @@ public void ShouldAppendSignedByte()
     [Fact]
     public void ShouldAppendMultipleChars()
     {
-        var builder = new ValueStringBuilder();
+        using var builder = new ValueStringBuilder();
 
         for (var i = 0; i < 64; i++)
         {
@@ -193,7 +193,7 @@ public void ShouldAppendMultipleChars()
     [Fact]
     public void ShouldAppendMultipleDoubles()
     {
-        var builder = new ValueStringBuilder();
+        using var builder = new ValueStringBuilder();
 
         builder.Append(1d / 3d);
         builder.Append(1d / 3d);

tests/LinkDotNet.StringBuilder.UnitTests/ValueStringBuilder.AppendJoin.Tests.cs

@@ -9,7 +9,7 @@ public class ValueStringBuilderAppendJoinTests
     [MemberData(nameof(StringSeparatorTestData))]
     public void ShouldAppendWithStringSeparator(string separator, IEnumerable<string?> values, string expected)
     {
-        var stringBuilder = new ValueStringBuilder();
+        using var stringBuilder = new ValueStringBuilder();
 
         stringBuilder.AppendJoin(separator, values);
 
@@ -20,7 +20,7 @@ public void ShouldAppendWithStringSeparator(string separator, IEnumerable<string
     [MemberData(nameof(CharSeparatorTestData))]
     public void ShouldAppendWithCharSeparator(char separator, IEnumerable<string?> values, string expected)
     {
-        var stringBuilder = new ValueStringBuilder();
+        using var stringBuilder = new ValueStringBuilder();
 
         stringBuilder.AppendJoin(separator, values);
 
@@ -30,7 +30,7 @@ public void ShouldAppendWithCharSeparator(char separator, IEnumerable<string?> v
     [Fact]
     public void ShouldAddDataWithStringSeparator()
     {
-        var stringBuilder = new ValueStringBuilder();
+        using var stringBuilder = new ValueStringBuilder();
 
         stringBuilder.AppendJoin(",", new object[] { 1, new DateTime(1900, 1, 1) });
 
@@ -40,7 +40,7 @@ public void ShouldAddDataWithStringSeparator()
     [Fact]
     public void ShouldAddDataWithCharSeparator()
     {
-        var stringBuilder = new ValueStringBuilder();
+        using var stringBuilder = new ValueStringBuilder();
 
         stringBuilder.AppendJoin(',', new object[] { 1, new DateTime(1900, 1, 1) });