Capture RenderBatch bytes synchronously. Fixes #1223

src/Microsoft.AspNetCore.Blazor.Server/Circuits/MessagePackBufferStream.cs

@@ -8,28 +8,19 @@ using System.IO;
 namespace Microsoft.AspNetCore.Blazor.Server.Circuits
 {
     /// <summary>
-    /// A write-only stream that outputs its data to an underlying expandable
+    /// Provides Stream APIs for writing to a MessagePack-supplied expandable buffer.
-    /// buffer in the format for a MessagePack 'Bin32' block. Supports writing
-    /// into buffers up to 2GB in length.
     /// </summary>
-    internal class MessagePackBinaryBlockStream : Stream
+    internal class MessagePackBufferStream : Stream
     {
-        // MessagePack Bin32 block
-        // https://github.com/msgpack/msgpack/blob/master/spec.md#bin-format-family
-        const int HeaderLength = 5;
-
         private byte[] _buffer;
         private int _headerStartOffset;
         private int _bodyLength;
 
-        public MessagePackBinaryBlockStream(byte[] buffer, int offset)
+        public MessagePackBufferStream(byte[] buffer, int offset)
         {
             _buffer = buffer ?? throw new ArgumentNullException(nameof(buffer));
             _headerStartOffset = offset;
             _bodyLength = 0;
-
-            // Leave space for header
-            MessagePackBinary.EnsureCapacity(ref _buffer, _headerStartOffset, HeaderLength);
         }
 
         public byte[] Buffer => _buffer;
@@ -38,8 +29,8 @@ namespace Microsoft.AspNetCore.Blazor.Server.Circuits
         public override bool CanSeek => false;
         public override bool CanWrite => true;
 
-        // Length is the complete number of bytes being output, including header
+        // Length is the complete number of bytes being output
-        public override long Length => _bodyLength + HeaderLength;
+        public override long Length => _bodyLength;
 
         // Position is the index into the writable body (i.e., so position zero
         // is the first byte you can actually write a value to)
@@ -65,24 +56,10 @@ namespace Microsoft.AspNetCore.Blazor.Server.Circuits
 
         public override void Write(byte[] src, int srcOffset, int count)
         {
-            var outputOffset = _headerStartOffset + HeaderLength + _bodyLength;
+            var outputOffset = _headerStartOffset + _bodyLength;
             MessagePackBinary.EnsureCapacity(ref _buffer, outputOffset, count);
             System.Buffer.BlockCopy(src, srcOffset, _buffer, outputOffset, count);
             _bodyLength += count;
         }
-
-        public override void Close()
-        {
-            // Write the header into the space we reserved at the beginning
-            // This format matches the MessagePack spec
-            unchecked
-            {
-                _buffer[_headerStartOffset] = MessagePackCode.Bin32;
-                _buffer[_headerStartOffset + 1] = (byte)(_bodyLength >> 24);
-                _buffer[_headerStartOffset + 2] = (byte)(_bodyLength >> 16);
-                _buffer[_headerStartOffset + 3] = (byte)(_bodyLength >> 8);
-                _buffer[_headerStartOffset + 4] = (byte)(_bodyLength);
-            }
-        }
     }
 }

src/Microsoft.AspNetCore.Blazor.Server/Circuits/RemoteRenderer.cs

@@ -3,8 +3,10 @@
 
 using System;
 using System.Threading.Tasks;
+using MessagePack;
 using Microsoft.AspNetCore.Blazor.Components;
 using Microsoft.AspNetCore.Blazor.Rendering;
+using Microsoft.AspNetCore.Blazor.Server.Circuits;
 using Microsoft.AspNetCore.SignalR;
 using Microsoft.JSInterop;
 
@@ -87,7 +89,15 @@ namespace Microsoft.AspNetCore.Blazor.Browser.Rendering
         /// <inheritdoc />
         protected override void UpdateDisplay(in RenderBatch batch)
         {
-            var task = _client.SendAsync("JS.RenderBatch", _id, batch);
+            // Send the render batch to the client
+            // Note that we have to capture the data as a byte[] synchronously here, because
+            // SignalR's SendAsync can wait an arbitrary duration before serializing the params.
+            // The RenderBatch buffer will get reused by subsequent renders, so we need to
+            // snapshot its contents now.
+            // TODO: Consider using some kind of array pool instead of allocating a new
+            //       buffer on every render.
+            var batchBytes = MessagePackSerializer.Serialize(batch, RenderBatchFormatterResolver.Instance);
+            var task = _client.SendAsync("JS.RenderBatch", _id, batchBytes);
             CaptureAsyncExceptions(task);
         }
 

src/Microsoft.AspNetCore.Blazor.Server/Circuits/RenderBatchFormatterResolver.cs

@@ -5,7 +5,6 @@ using MessagePack;
 using MessagePack.Formatters;
 using Microsoft.AspNetCore.Blazor.Rendering;
 using System;
-using System.IO;
 
 namespace Microsoft.AspNetCore.Blazor.Server.Circuits
 {
@@ -16,6 +15,8 @@ namespace Microsoft.AspNetCore.Blazor.Server.Circuits
     /// </summary>
     internal class RenderBatchFormatterResolver : IFormatterResolver
     {
+        public static readonly RenderBatchFormatterResolver Instance = new RenderBatchFormatterResolver();
+
         public IMessagePackFormatter<T> GetFormatter<T>()
             => typeof(T) == typeof(RenderBatch) ? (IMessagePackFormatter<T>)RenderBatchFormatter.Instance : null;
 
@@ -30,17 +31,17 @@ namespace Microsoft.AspNetCore.Blazor.Server.Circuits
             public int Serialize(ref byte[] bytes, int offset, RenderBatch value, IFormatterResolver formatterResolver)
             {
                 // Instead of using MessagePackBinary.WriteBytes, we write into a stream that
-                // knows how to format its output as a MessagePack binary block. The benefit
+                // knows how to write the data using MessagePack writer APIs. The benefit
                 // is that we don't have to allocate a second large buffer to capture the
                 // RenderBatchWriter output - we can just write directly to the underlying
                 // output buffer.
-                using (var binaryBlockStream = new MessagePackBinaryBlockStream(bytes, offset))
+                using (var bufferStream = new MessagePackBufferStream(bytes, offset))
-                using (var renderBatchWriter = new RenderBatchWriter(binaryBlockStream, leaveOpen: false))
+                using (var renderBatchWriter = new RenderBatchWriter(bufferStream, leaveOpen: false))
                 {
                     renderBatchWriter.Write(value);
 
-                    bytes = binaryBlockStream.Buffer; // In case the buffer was expanded
+                    bytes = bufferStream.Buffer; // In case the buffer was expanded
-                    return (int)binaryBlockStream.Length;
+                    return (int)bufferStream.Length;
                 }
             }
         }

src/Microsoft.AspNetCore.Blazor.Server/DependencyInjection/ServerSideBlazorServiceCollectionExtensions.cs

@@ -126,10 +126,7 @@ namespace Microsoft.Extensions.DependencyInjection
             // method on ISignalRServerBuilder so the developer always has to chain it onto
             // their own AddSignalR call. For now we're keeping it like this because it's
             // simpler for developers in common cases.
-            services.AddSignalR().AddMessagePackProtocol(options =>
+            services.AddSignalR().AddMessagePackProtocol();
-            {
-                options.FormatterResolvers.Insert(0, new RenderBatchFormatterResolver());
-            });
         }
     }
 }

test/Microsoft.AspnetCore.Blazor.Server.Test/Circuits/MessagePackBufferStreamTest.cs

@@ -1,41 +1,25 @@
 // Copyright (c) .NET Foundation. All rights reserved.
 // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
 
-using MessagePack;
 using Microsoft.AspNetCore.Blazor.Server.Circuits;
 using System;
 using Xunit;
 
 namespace Microsoft.AspNetCore.Blazor.Server
 {
-    public class MessagePackBinaryBlockStreamTest
+    public class MessagePackBufferStreamTest
     {
         [Fact]
         public void NullBuffer_Throws()
         {
             var ex = Assert.Throws<ArgumentNullException>(() =>
             {
-                new MessagePackBinaryBlockStream(null, 0);
+                new MessagePackBufferStream(null, 0);
             });
 
             Assert.Equal("buffer", ex.ParamName);
         }
 
-        [Fact]
-        public void WithNoWrites_JustOutputsHeader()
-        {
-            // Arrange
-            var buffer = new byte[100];
-            var offset = 58; // Arbitrary
-
-            // Act
-            new MessagePackBinaryBlockStream(buffer, offset).Dispose();
-
-            // Assert
-            Assert.Equal(MessagePackCode.Bin32, buffer[offset]);
-            Assert.Equal(0, ReadBigEndianInt32(buffer, offset + 1));
-        }
-
         [Fact]
         public void WithWrites_WritesToUnderlyingBuffer()
         {
@@ -44,32 +28,30 @@ namespace Microsoft.AspNetCore.Blazor.Server
             var offset = 58; // Arbitrary
 
             // Act/Assert
-            using (var stream = new MessagePackBinaryBlockStream(buffer, offset))
+            using (var stream = new MessagePackBufferStream(buffer, offset))
             {
                 stream.Write(new byte[] { 10, 20, 30, 40 }, 1, 2); // Write 2 bytes
                 stream.Write(new byte[] { 101 }, 0, 1); // Write another 1 byte
                 stream.Close();
 
-                Assert.Equal(MessagePackCode.Bin32, buffer[offset]);
+                Assert.Equal(20, buffer[offset]);
-                Assert.Equal(3, ReadBigEndianInt32(buffer, offset + 1));
+                Assert.Equal(30, buffer[offset + 1]);
-                Assert.Equal(20, buffer[offset + 5]);
+                Assert.Equal(101, buffer[offset + 2]);
-                Assert.Equal(30, buffer[offset + 6]);
-                Assert.Equal(101, buffer[offset + 7]);
             }
         }
 
         [Fact]
-        public void LengthIncludesHeaderButPositionDoesNot()
+        public void LengthAndPositionAreEquivalent()
         {
             // Arrange
             var buffer = new byte[20];
             var offset = 3;
 
             // Act/Assert
-            using (var stream = new MessagePackBinaryBlockStream(buffer, offset))
+            using (var stream = new MessagePackBufferStream(buffer, offset))
             {
                 stream.Write(new byte[] { 0x01, 0x02 }, 0, 2);
-                Assert.Equal(7, stream.Length);
+                Assert.Equal(2, stream.Length);
                 Assert.Equal(2, stream.Position);
             }
         }
@@ -78,15 +60,15 @@ namespace Microsoft.AspNetCore.Blazor.Server
         public void WithWrites_ExpandsBufferWhenNeeded()
         {
             // Arrange
-            var origBuffer = new byte[15];
+            var origBuffer = new byte[10];
             var offset = 6;
             origBuffer[0] = 123; // So we can check it was retained during expansion
 
             // Act/Assert
-            using (var stream = new MessagePackBinaryBlockStream(origBuffer, offset))
+            using (var stream = new MessagePackBufferStream(origBuffer, offset))
             {
-                // We can fit the 6-byte offset plus 5-byte header plus 3 written bytes
+                // We can fit the 6-byte offset plus 3 written bytes
-                // into the original 15-byte buffer
+                // into the original 10-byte buffer
                 stream.Write(new byte[] { 10, 20, 30 }, 0, 3);
                 Assert.Same(origBuffer, stream.Buffer);
 
@@ -98,13 +80,11 @@ namespace Microsoft.AspNetCore.Blazor.Server
                 // Check the expanded buffer has the expected contents
                 stream.Close();
                 Assert.Equal(123, stream.Buffer[0]); // Retains other values from original buffer
-                Assert.Equal(MessagePackCode.Bin32, stream.Buffer[offset]);
+                Assert.Equal(10, stream.Buffer[offset]);
-                Assert.Equal(5, ReadBigEndianInt32(stream.Buffer, offset + 1));
+                Assert.Equal(20, stream.Buffer[offset + 1]);
-                Assert.Equal(10, stream.Buffer[offset + 5]);
+                Assert.Equal(30, stream.Buffer[offset + 2]);
-                Assert.Equal(20, stream.Buffer[offset + 6]);
+                Assert.Equal(40, stream.Buffer[offset + 3]);
-                Assert.Equal(30, stream.Buffer[offset + 7]);
+                Assert.Equal(50, stream.Buffer[offset + 4]);
-                Assert.Equal(40, stream.Buffer[offset + 8]);
-                Assert.Equal(50, stream.Buffer[offset + 9]);
             }
         }