ArgumentOutOfRangeException in PagedWriterBuffer

[CoderNate]

I'm getting the following error:

System.ArgumentOutOfRangeException: Specified argument was out of the range of valid values.
Parameter name: start
   at System.ThrowHelper.ThrowArgumentOutOfRangeException(ExceptionArgument argument)
   at Amazon.IonDotnet.Internals.PagedWriterBuffer.WriteBytes(ReadOnlySpan`1 bytes)
   at Amazon.IonDotnet.Internals.PagedWriterBuffer.WriteUtf8(ReadOnlySpan`1 chars, Int32 length)
   at Amazon.IonDotnet.Internals.Binary.RawBinaryWriter.WriteString(String value)

In the constructor the blockSize gets set like this:

        protected PagedWriterBuffer(int intendedBlockSize)
        {
            ...
            this.currentBlock = ArrayPool<byte>.Shared.Rent(intendedBlockSize);
            ...
            this.blockSize = this.currentBlock.Length;
        }

Note that the ArrayPool can return a larger array than what you ask for. Later on currentBlock gets replaced in AllocateNewBlock:

        private void AllocateNewBlock() {
            ...
            var newBlock = ArrayPool<byte>.Shared.Rent(this.blockSize);
            this.bufferBlocks.Add(newBlock);
            this.currentBlock = newBlock;
        }

What I'm running into is:

1. The blockSize is set to 512 in the constructor and currentBlock starts out having length 512
2. Later in AllocateNewBlock the ArrayPool returns a block that's 1024 and it's assigned to currentBlock
3. The WriteUtf8 method writes bytes as long as it doesn't go past this.currentBlock.Length which is 1024
4. The WriteBytes method looks at blockSize instead of currentBlock.Length:

        public void WriteBytes(ReadOnlySpan<byte> bytes)
        {
            ...
            var bytesToWrite = bytes.Length;
            while (bytesToWrite > 0)
            {
                // If blockSize is 512 and runningIndex is greater than 512 then "left" will be negative:
                var left = this.blockSize - this.runningIndex;  
                var bytesWritten = bytesToWrite > left ? left : bytesToWrite;
                // A negative number for bytesWritten causes the following to throw the ArgumentOutOfRangeException
                bytes.Slice(0, bytesWritten).CopyTo(new Span<byte>(this.currentBlock, this.runningIndex, bytesWritten));
                ...
            }
        }

I assume that WriteBytes should refer to currentBlock.Length like all the other methods do instead of blockSize. Perhaps the blockSize field should be removed. However, AllocateNewBlock needs to know what size to pass to ArrayPool.Rent so maybe the intendedBlockSize argument from the constructor should be stored in a readonly field for that purpose in place of using blockSize.

Thanks for making this library available!

[CoderNate]

I can consistently reproduce this in .NET framework 4.8 with the code below running as a console app. It doesn't appear to happen in .NET core 3.1.

    class Program
    {
        static void Main(string[] args)
        {
            Console.WriteLine(System.Runtime.InteropServices.RuntimeInformation.FrameworkDescription);
            for (var i = 0; i < 20; i++)
            {
                RunTest();
            }
        }
        static void RunTest()
        {
            using var ms = new System.IO.MemoryStream();
            using var writer = Amazon.IonDotnet.Builders.IonBinaryWriterBuilder.Build(ms);
            var r = new Random(0); // Always use the same seed so we get the same sequence of values out
            writer.StepIn(Amazon.IonDotnet.IonType.List);
            for (var i = 0; i < 100_000; i++)
            {
                var strLen = r.Next(10, 40);
                try
                {
                    writer.WriteString(new string('a', strLen));
                }
                catch (ArgumentOutOfRangeException ex)
                {
                    Console.WriteLine($"Exception on i == {i}");
                    return;
                }
            }
        }
    }

This produces:

.NET Framework 4.8.4510.0
Exception on i == 979
Exception on i == 979
Exception on i == 979
Exception on i == 979
Exception on i == 979
Exception on i == 979
Exception on i == 979
Exception on i == 979
Exception on i == 979
Exception on i == 979
Exception on i == 979
Exception on i == 979
Exception on i == 979
Exception on i == 979
Exception on i == 979
Exception on i == 979
Exception on i == 979
Exception on i == 979
Exception on i == 979
Exception on i == 979

I got this same result on a second machine running .NET 4.8.4110.0. I don't know if this could reliably be made into a unit test though because the bug is triggered by using the ArrayPool that's shared by all threads.

[tgregg]

The related fix has been merged; keeping this open so we can consider adding repeatable tests for this case.